-
Notifications
You must be signed in to change notification settings - Fork 279
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
base: master
Are you sure you want to change the base?
Conversation
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 What do you think? |
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:
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. |
I thought that all these would fit into the 'example' VC:
The minimum template would fit in
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.
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.
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.
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.
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:
|
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.
Exactly.
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... |
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: 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. |
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 |
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'. |
Yes, all I need now is functionality for generating a compliance test suite for a specific VC and then create a |
Minor addition, but I think there should also be functions to retrieve the actor, logger, etc. from the VC. Should they be named |
bffd5e1
to
5eb236f
Compare
e853c6a
to
0c7dfc1
Compare
b7bd30a
to
6952874
Compare
… warning if that is so. Using the same default logger for many VC makes it hard to trace the origin of a log entry.
Improved compliance tests to detect lack of as_sync function and added missing functions to VUnit VCIs
6952874
to
883d975
Compare
@LarsAsplund, see #758, which fixes most of the linting/format/f-string issues. |
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 reportingRationale: 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 checkingRationale: 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 communicationRationale: 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 bedefault_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 bedefault_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 toget_std_cfg
Rationale: Makes it possible to reuse operations such as
get_logger
between VCsRule: 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:
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
andVerificationComponent
classes. These classes provides methods to generate the full VHDL testbenches from the templates and some extra parameters. See therun.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