-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Split check_test_cases.py and outcome_analysis.py #9668
Split check_test_cases.py and outcome_analysis.py #9668
Conversation
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Move the test case collection code out of check_test_cases.py and into its own module. This allows outcome analysis to depend only on the new module and not on check_test_cases.py. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
…mes.py Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
The ignore list for coverage only has two test cases out of ~10000 that are currently reported as not executed. This is a drop in the sea and not useful. Remove them so that the class can be used generically. A follow-up will construct a comprehensive ignore list. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Use different names for task name, a task class and a task instance. The interpreter doesn't care, but it's less confusing for both humans and type checkers. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Always have tasks_list be a list, not potentially some fancier iterable. Bypass mypy's somewhat legitimate complaint about REFERENCE and DRIVER in task_class: they could potentially be instance attributes, but we rely on them being class attributes. Python does normally guarantee their existence as class attributes (unless a derived class explicitly deletes them), but they could be overridden by an instance attribute; that's just something we don't do, so the class attribute's value is legitimate. We can't expect mypy to know that, so work around its complaint. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Place the code of outcome analysis (auxiliary functions, tasks, command line entry point) into a separate module, which will be moved to the version-independent framework repository so that it can be shared between maintained branches. Keep the branch-specific list of driver components and ignore lists in the per-repository script. We keep the executable script at `tests/scripts/analyze_outcomes.py`. It's simpler that way, because that path is hard-coded in CI scripts. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
d85c6ce
to
da961e7
Compare
da961e7
to
5d2d872
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Only two minor comments.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Move `collect_test_cases.py` (split from `check_test_cases.py`), `check_test_cases.py`, and `outcome_analysis.py` (split from `analyze_outcomes.py`) to the framework repository. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Upgrade mypy to 0.971, which is the last version that supports Python 3.6 (the oldest Python version that we currently run on the CI). This fixes the error ``` framework/scripts/mbedtls_framework/outcome_analysis.py:119: error: Incompatible return value type (got "IO[Any]", expected "TextIO") framework/scripts/mbedtls_framework/outcome_analysis.py:121: error: Incompatible return value type (got "IO[Any]", expected "TextIO") ``` As far as I can tell the fix is python/mypy#9275 which was released in mypy 0.940. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
mypy >=0.960 rejects macro_collector.py. Mbed-TLS/mbedtls-framework#50 We currently need mypy >=0.940, <0.960. Pick 0.942, which works, and is the system version on Ubuntu 22.04. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Currently, many test cases are not executed. A follow-up pull request will take care of that. In the meantime, continue allowing partial test coverage. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
5d2d872
to
5ee987d
Compare
I have rewritten the history a little so that there is now a single commit that removes the files moved to the framework, obtained with David's script. I've also updated the framework submodule to the latest version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-approving after force-push - content is identical to what I previously approved, with the exception of the framework pointer updated to the latest version of Mbed-TLS/mbedtls-framework#55
…_analysis.py Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
5ee987d
to
8fa4964
Compare
I just amended the last commit to switch the framework to the result of merging the companion PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after updating the framework pointer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but the CI is unhappy
The CI failure looks infrastructure-related:
This might just be a network glitch, I'll try re-running the job. |
80352ac
check_test_cases.py
into test case collection (collect_test_cases.py
, used by outcome analysis) and the checks proper. Move both parts to the framework.analyze_outcomes.py
into code (outcome_analysis.py
, moved to the framework) and data (analyze_outcomes.py
, stays in its branch).outcome_analysis.py
.PR checklist