Add compliance tests and updated VCs accordingly

[LarsAsplund]

This PR provides compliance testing of VUnit VCs and VCIs according to the following rules:

Rule: The file containing the VCI package shall only contain one package Rationale: The VCI can be referenced by file name which makes compliance testing simpler

Rule: The name of the function used to create a new instance handle for a VC, aka the constructor, shall start with new_. Rationale: Makes it easier for the compliance test to automatically find a constructor and evaluate that with respect to the other applicable rules

Rule: A VC constructor shall have a logger parameter giving the user the option to specify the logger to use for VC reporting Rationale: It enables user control of the logging, for example enabling debug messages.

Rule: A VC constructor shall have a checker parameter giving the user the option to specify the checker to use for VC checking Rationale: It enables user control of the checking, for example setting properties like the stop-level (without affecting the logger used above)

Rule: A VC constructor shall have an actor parameter giving the user the option to specify the actor to use for VC communication Rationale: It enables user control of the communication, for example setting name for better trace logs.

Rule: A VC constructor shall have an unexpected_msg_type_policy parameter giving the user the option to specify the action taken when the VC receives an unexpected message type. Rationale: A VC actor setup to subscribe to another actor may receive messages not relevant for its operation. OTOH, VCs just addressed directly should only recieve messages it can handle.

Rule: A VC constructor shall have a default value for all required parameters above. Rationale: Makes it easier for the user if there is no preference on what to use.

Rule: The default value for the logger parameter shall not be default_logger Rationale: Using a logger more associated with the VC makes the logger output easier to understand

Rule: The default value for the checker parameter shall not be default_checker Rationale: Using a checker more associated with the VC makes the checker output easier to understand

Rule: All fields in the handle returned by the constructor shall start with p_. Rationale: All field shall be considered private and this is a way to emphasize this. Keeping them private makes updates easier without breaking backward compatibility

Rule: The standard configuration (std_cfg_t) of a VC consisting of the required parameters to the constructor shall be possible to get from the handle using a call to get_std_cfg Rationale: Makes it possible to reuse operations such as get_logger between VCs

Rule: The file containing the VC entity shall only contain one entity Rationale: The VC can be referenced by file name which makes compliance testing simpler

Rule: A VC shall only have one generic Rationale: Representing a VC with a single object makes it easier to handle in code. Since all fields of the handle are private future updates have less risk of breaking backward compatibility

Rule: All VCs shall support the sync interface. Rationale: Being able to check that a VC is idle and to add a delay between transactions are commonly useful operations for VC users. A side effect is that some of these rules are more easily checked by the compliance tool.

Compliance testing is done separately for the VC and the VCI and each test consists of two parts. One part test the code by parsing it and the other part tests the code by running a VHDL testbench.

The VHDL testbenches cannot be automatically created because of:

• A VC constructor can have VC specific parameters without default values
• A VC port list can constain unconstrained arrays

These issues are solved by creating a templates for the VHDL testbenches. The template is created by calling the compliance_test.py script (--help for instruction). The generated templates are then modified according to the embedded instructions to add the missing default values and constrain the unconstrained ports.

The run.py script for the VC and VCI can then add compliance tests by using the VerificationComponentInterface and VerificationComponent classes. These classes provides methods to generate the full VHDL testbenches from the templates and some extra parameters. See the run.py file for the VUnit provided verification components.

All VUnit provided verification components have been updated to comply with the rules. For some of them it means breaking backward compatibility

[eine]

I think that this conflicts with #511, in the sense that vc_uart or vc_axi are expected to be references/templates for anyone willing to contribute with a third-party VC.

Hence, I think that this PR could be a new repo (say VUnit/vc_foo_bar), which would be the reference and only the reference, instead of vc_uart and vc_axi which will be actual usable VCs. The only requirement so far is to use subdir examples for examples, src for sources, and to add a minimal vunit_cfg.json. But this is being discussed yet.

What do you think?

[LarsAsplund]

I think in the long run it will be good to separate what defines a good VC from the actual VCs since the template VC project will have another focus. Good examples are important but there is also a question about providing a minimum template, rationale for design patterns, tutorials, compliance tests etc. I also think the template is owned by the core project since what's good design partly relates to what the core provides in terms of VC functionality. Ideally, I would like the VUnit core to provide a compliance test that could evaluate a VC given the repo root. Initially the compliance test can be extremely simple but it will give us a good starting platform:

• Anyone can evaluate a VC and suggest or contribute to a project to make it improve
• Our web site can not only provide links to projects but also maintain an up-to-date "score"
• We can provide badges related to that score
• By making the compliance test based on the repo root it may even be possible to test if the repo is using the badge and exposing their score.
• ...

If we're are in control of the rules for the "game" we can design it such that people strive for high quality without us having to actively enforcing it.

So a new repo is probably good but maybe the capability to run the compliance test should be part of the core and present in any installation.

[eine]

Good examples are important but there is also a question about providing a minimum template, rationale for design patterns, tutorials, compliance tests etc.

I thought that all these would fit into the 'example' VC:

- docs
- examples
  - test_examples.py
  - example_a
  - example_b
  - ...
- src
  - vc_*_context.vhd
  - *_pkg.vhd
  - ...
- test
  - tb_*.vhd
- .gitattributes
- .gitignore
- .travis.yml
- run.py
- tox.ini
- vunit_cfg.json

The minimum template would fit in src, tutorials in examples and compliance tests in test. The rationale for design patterns can be contained in docs (*.rst or *.md) and/or in src (comments in sources). Note that:

• The 'examples' subdir can be automatically integrated into the website by extending #516. From the perspective of the user, all the examples (the ones in the main repo, and those in all the 'supported/known' VC repos are listed together -separated in subsections-).
• The docs can contain a 'dummy' configuration file and index, so that all the content can be integrated in VUnit's site. But, it is possible to publish exactly the same content to it's own site at the same time. Maybe, we should define a subdir inside docs which is to be integrated in the main web site, while other documentation sources are expected to be independent.
• .travis.yml, tox.ini, run.py and test_examples.py provide the base for a consistent testing setup. I.e., if desired, VUnit that run the test suited.

I also think the template is owned by the core project since what's good design partly relates to what the core provides in terms of VC functionality.

I think that 'owning' this part of the project does not imply it being part of the main repo. 'Own' relates to licensing and permissions, not not location.

Overall, I am ok with keeping some parts or everything in the main repo, that's your decission. But thinking on the UX of a potential contributor willing to create a new VC in a separate repo, I think it is easier to clone a dummy/reference repo and focus on the documentation/examples/descriptions in it; instead of having to cherry-pick the relevant sources from the main repo.

Ideally, I would like the VUnit core to provide a compliance test that could evaluate a VC given the repo root.

So a new repo is probably good but maybe the capability to run the compliance test should be part of the core and present in any installation.

Yes, I think that these compliance tests should be part of the main repo. It would allow third-party VC repos to use them on their own, in order to evaluate their status. It would also allow VUnit to test the compliance with no regard to the tests/examples available in the third-party repo.

Our web site can not only provide links to projects but also maintain an up-to-date "score"

The 'logic' to extract examples (list and docstrings) and the list of VCs provided by each VC repo is ready: https://github.com/1138-4EB/vunit/blob/site-rework/tools/docs_utils.py#L100-L210 It would be straighforward to extract other metadata, or to 'generate' it.

There is no PR related to this yet, becase #516 and #511 should be merged first. BTW, could you please have a look at #516? I think it is interesting by itself, independently of all these VC-related changes.

We can provide badges related to that score

I know how to dynamically generate badges for the docs (Sphinx). However, I don't know how third-party repos can integrate them in their README's and have them updated automatically. We could probably serve a JSON files and use the 'Dynamic' option in https://shields.io/; but I have never tried it.

By making the compliance test based on the repo root it may even be possible to test if the repo is using the badge and exposing their score.

I don't understand why we would want to test this. As long as we can test the repo and provide it a score/badge in VUnit's web site, we should not care about it being shown in the README also, should we? Is this some kind of 'social rating'?

EDIT

Example of how information from third-party VCs is integrated in the main site:

• https://1138-4eb.github.io/vunit/examples.html#verification-components
• https://1138-4eb.github.io/vunit/verification_components/user_guide.html#verification-components

Notes:

• Note the references/links.
• Ignore the style/theme of the site. It is unrelated to the content.

[LarsAsplund]

I think that 'owning' this part of the project does not imply it being part of the main repo. 'Own' relates to licensing and permissions, not not location.

I'm all for a separate repo. By ownership I was more referring to responsibility. The responsibility for maintaining best practices should not be with a specific VC. UART or any other VC will never care about all VC use cases nor use all of the features that VUnit provides for VC building.

Yes, I think that these compliance tests should be part of the main repo. It would allow third-party VC repos to use them on their own, in order to evaluate their status. It would also allow VUnit to test the compliance with no regard to the tests/examples available in the third-party repo.

Exactly.

I don't understand why we would want to test this. As long as we can test the repo and provide it a score/badge in VUnit's web site, we should not care about it being shown in the README also, should we? Is this some kind of 'social rating'?

I'm basically just pointing out that with a VUnit core owned compliance test based on the VC repo root we have a platform for creating whatever means we think necessary to create a high-quality and interoperable ecosystem. Not really saying that all of them are great. The first step is to have a "rating" system. To start with that would be a pass/fail compliance test. The second step is to make that rating as visible as possible. The more visible a score is the more motivation to keep it high. Personally I care a lot more about the quality badges on vunit.github.io than what other ranking sites may say about the project. I'm thinking that a VC maintainer would care more about the quality metrics presented in her repo than the ones presented on vunit.github.io. Enforcing a badge in the README might be a bit too much though...

[eine]

UART or any other VC will never care about all VC use cases nor use all of the features that VUnit provides for VC building.

As a matter of fact, there is one example for UART, two for AXI and none for Avalon or Wishbone. The number of tests is also quite different. None of them has any specific documentation.

Regarding the rating/badges, gotcha, and I agree. I can try a first iteration by creating a table with three fields per VC: 'tests pass', 'has examples' and 'has docs'. The rating might be:

• when tests + egs + docs
• when tests [+ egs] [+ docs]
• when !tests

Examples, if exist, must also pass/run.

I'd like to know what are your ideas for integrating these new contents (table, per VC docs, per VC examples, etc.) in the web site. Do you have any plan to (re)organize the documentation about verification components?

As you can see in https://1138-4eb.github.io/vunit/, I moved 'Examples' out of the 'User Guide', but VC are still in Documentation > VHDL Libraries > Verification Components. Furthermore, since the sidebar is different, the number of clicks to reach from the home to the Verification Components is reduced from 3 to 2. This might sound stupid, by I find myself having to browse that section quite frequently, and it annoys me. Nonetheless, these are just tests.

[LarsAsplund]

My first priority right now would be to have some basic use cases in the template. Blocking/non-blocking transactions and transactions with or without a reply. Based on our discussions I think the next step would be to create a template repository and then the mechanism allowing such a repo to be evaluated from a VUnit core installation

[eine]

Based on our discussions I think the next step would be to create a template repository and then the mechanism allowing such a repo to be evaluated from a VUnit core installation

Since vc_axi and vc_uart are created already, I was thinking about doing the mechanism to evaluate them, while the template is being completed. I believe that the 'evaluation' will be based on running some python tests. Therefore, from the point of view of the mechanism involving Git + Travis + Sphinx + Pages, the specific names of the tests do not matter. Anyway, I'll wait until this is ready to be split to a 'template repo'.

[LarsAsplund]

I believe that the 'evaluation' will be based on running some python tests.

Yes, all I need now is functionality for generating a compliance test suite for a specific VC and then create a run.py for that

[bradleyharden]

Minor addition, but I think there should also be functions to retrieve the actor, logger, etc. from the VC. Should they be named actor, logger, etc. or as_actor, get_logger, etc.?

[eine]

Ref: https://help.github.com/en/articles/creating-a-template-repository

[eine]

Apart from the in-code comments above, some other preliminary points:

• Can we merge the rules about having a single package/entity in a single file?
• Can we have the rules numbered or assigned any other identifier?
• I think it would be cleaner to define the templates in test_compliance_test.py as separate *.vhd.tpl files.
• We should avoid to use dirname, join, etc. in new code, and use from pathlib import Path instead.

[LarsAsplund]

Apart from the in-code comments above, some other preliminary points:

• Can we merge the rules about having a single package/entity in a single file?

Yes-

• Can we have the rules numbered or assigned any other identifier?

Yes. I think I'll move all that to the user guide as a start for the documentation

• I think it would be cleaner to define the templates in test_compliance_test.py as separate *.vhd.tpl files.

I'll try to move it out of the way somewhere

• We should avoid to use dirname, join, etc. in new code, and use from pathlib import Path instead.

I guess it's time to abandon old habits

[LarsAsplund]

Minor addition, but I think there should also be functions to retrieve the actor, logger, etc. from the VC. Should they be named actor, logger, etc. or as_actor, get_logger, etc.?

@bradleyharden Right now I've bundled all standard configuration items in a record and I require that every VC can provide that when called with get_std_cfg. If that is in place you can reuce the getters. I.e. get_logger(get_std_cfg(handle)). A VC designer could of course provide wrappers as a convenience but I'm thinking that a standard configuration would allow us to make (minor) updates that would be available to all VCs without any updates on the VC side.

[LarsAsplund]

I've pushed an updated version to address the comments I received.

[LarsAsplund]

Since we're doing some backward compatibility breaking changes it would be good the hear what some of the other contributors have to say. @slaweksiluk @eschmidscs @dstadelm

[eschmidscs]

@LarsAsplund Sorry, didn't see your comment until now (actually @dstadelm had to point it out).

I currently don't grasp the full change. What you discuss here sounds feasible to me - and is yours to decide in the end.

Concerning backwards compatibility: It seems to me that this is not a major blocker, certainly some fixes. Am I wrong? Can I just merge this PR locally into master and check what happens?

I'm generally happy if I know ahead that something will break instead of figuring out that something broke (as has happened with the non-blocking axi_stream check). It is normally not too hard to fix once you have a time slot available.

[eschmidscs]

Any chance I can help to get this one forward?

[eine]

@eschmidscs, this PR was just rebased and brought up to date. Please, check it out, and run the compliance tests with your simulators, and/or use your usual designs/tests. Report any issue you find. Moreover, if you want to actually review the content, that will be welcome too. If any of your PRs is affected by this one, you might want to rebase them on top. Yet, beware that this might be further rebased, so I suggest you keep a backup of yours.

[eschmidscs]

It would be helpful if the compliance_test.py would add a correct copyright header.

[eschmidscs]

I am trying to get #703 to run with the compliance checks. Given the issues already noted, it seems I am getting somewhere.

The show stopper right now is the p_initial that I added to the handle, currently an unconstrained std_logic_vector. It is unconstrained in analogy to the std_logic_checker where the value port is unconstrained, thus the std_logic_driver should have it unconstrained as well, resulting in p_initial being unconstrained.

The unconstrained vector cannot work with the compliance testbenches, because they don't constrain the handle. ~~The easy fix is to add a length parameter to the handle and have all vectors constrained. This is ok for me.~~ This is not correct, I already failed once down that road: It is not allow to use a generic as length of a generic... That was the reason why I introduced the unconstrained vector.

But maybe you want to have unconstrained vectors working?

[LarsAsplund]

@eschmidscs I will have a test run of the compliance tests on your signal driver to see what conclusions I can make.

[LarsAsplund]

@eschmidscs I updated to support unconstrained handle fields. Please rebase and try with #703 again

[eschmidscs]

@LarsAsplund Ok, done this. Seems to work fine. Maybe you check #703 if the result is what you wanted.

It would be helpful if the compliance_test.py would add a correct copyright header.

This would still be handy. I kept running iterations of calling the script, patching back my changes and running the testbenches. If the copyright would already be there, the patch work would be reduced by 50%.

[LarsAsplund]

@eschmidscs The reason that no header is added is that the plan is for people to create independent repos for their VCs which are maintained by them under their license of choice. What we could do is to add the ability to specify a text file containing the header and then have the VUnit header as the default if no file is specified

[eschmidscs]

@LarsAsplund Ok, that's a reason. Could you go fancy and add both the additional file you propose and, if only the flag but no file is specified, just insert your standard header?

[LarsAsplund]

@eschmidscs That should be possible. We could also interpret the text file as an f-string. That would allow access to some convenient strings. For example,

{vc_header.vunit_license}
--
-- Copyright (c) {vc_header.year} John Doe, [email protected]

would result in

-- This Source Code Form is subject to the terms of the Mozilla Public -- License, v. 2.0. If a copy of the MPL was not distributed with this file, -- You can obtain one at http://mozilla.org/MPL/2.0/. -- -- Copyright (c) 2021, John Doe [email protected]

Or maybe a standard header function.

{vc_standard_header(name="John Doe", mail="[email protected]")}

That would give the same output as above. There would also be optional arguments to specify license and year.

[eschmidscs]

@LarsAsplund That looks nice! I like the f-string idea.

[umarcor]

@LarsAsplund, see #758, which fixes most of the linting/format/f-string issues.