Hypervisor updates: Mainly test coverage #49
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A protocoll class is never executed and contains only signatures of function. This exclude all classes that are derived only from Protocol from the coverage report
Jinja by-default strips all newlines at the end of the output. For conformance reasons, the last new line shouldn't be stripped
I.e. move the conversion to path out of the function. That simplifies the tests and increases readability.
There is no need to keep the parameter around. It is empty anyway.
modules is a set and the order of elements in a set is not derministic in python. So covnert it to a property and return a sorted list for rendering in jinja
* mark class variables as ClassVar * remove typo in invalid enum message * change type check for values to be exact instead of isinstance (bool is an instance of int, but should fail) * Report all missing keys instead of terminating for the first missing key * Make messages deterministic by sorting the values
Previously the incldues were parsed depth-first but bottom-top. I.e. the list in base in each file was parsed in the wrong and unintuitive order. In the example ``` bases: - file_a - file_b ``` One would expect a value that is set in file_a and file_b will be used from file_b, but it was the other way around.
The file exists and can very well be used if it is not overwritten by a specialization. This shouldn't do any harm, because all configurations that use the generator ship their own modules.list.j2
This excludes some lines from the coverage report, that just cannot be tested either because they are only possible in theory (and mypy would complain without a condition) or because they are only called in special szenarios (i.e. `if __name__ == "__main__":`)
|
I will test the changes again using the ebclfsa sample later. Please do not merge before I verified, that nothing is borken |
With #50 merged, this works fine -> Ready to merge |
MofX
force-pushed
the
hypervisor_updates
branch
2 times, most recently
from
b95c8b6 to
9171b6d
Compare
thir820
approved these changes
Feb 7, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Changes affecting everything:
Some minor function changes to the hypervisor generator:
Apert from that this PR mainly increase the test coverage for the hypervisor config generator to100%.
There are also some tests now, that do end-to-end testing. That is providing a config file and an extension folder and verifying the generated files against expected results.
Dependencies:
None
Tests results
Not part of robot tests
Developer Checklist:
Reviewer checklist: