Skip to content

Add compliance tests and updated VCs accordingly #520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

LarsAsplund
Copy link
Collaborator

@LarsAsplund LarsAsplund commented Jun 17, 2019

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
Copy link
Collaborator

eine commented Jun 17, 2019

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
Copy link
Collaborator Author

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
Copy link
Collaborator

eine commented Jun 18, 2019

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 feat(docs): generate docs/examples.rst automatically from docstrings #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:

Notes:

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

@LarsAsplund
Copy link
Collaborator Author

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
Copy link
Collaborator

eine commented Jun 18, 2019

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:

  • success when tests + egs + docs
  • important when tests [+ egs] [+ docs]
  • critical 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
Copy link
Collaborator Author

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
Copy link
Collaborator

eine commented Jun 18, 2019

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
Copy link
Collaborator Author

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
Copy link
Contributor

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
Copy link
Collaborator

eine commented Aug 9, 2019

@LarsAsplund LarsAsplund force-pushed the vc-template branch 10 times, most recently from bffd5e1 to 5eb236f Compare November 3, 2019 20:09
@LarsAsplund LarsAsplund force-pushed the vc-template branch 2 times, most recently from e853c6a to 0c7dfc1 Compare November 15, 2019 20:29
@umarcor
Copy link
Member

umarcor commented Oct 22, 2021

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

@eine eine modified the milestones: v5.0.0, v4.7.0 Oct 25, 2021
@eine eine modified the milestones: v5.0.0, v5x Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants