Skip to content

Add outcome analysis to TF-PSA-Crypto: framework support#292

Merged
gilles-peskine-arm merged 6 commits intoMbed-TLS:mainfrom
gilles-peskine-arm:analyze_outcomes-add_to_crypto-framework
Apr 8, 2026
Merged

Add outcome analysis to TF-PSA-Crypto: framework support#292
gilles-peskine-arm merged 6 commits intoMbed-TLS:mainfrom
gilles-peskine-arm:analyze_outcomes-add_to_crypto-framework

Conversation

@gilles-peskine-arm
Copy link
Copy Markdown
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Mar 26, 2026

Support for Mbed-TLS/TF-PSA-Crypto#729

  • In coverage analysis, distinguish between uncovered tests (which must not be executed) and ignored tests (which may or may not be executed).
  • During a transition period, consuming branches can keep defining IGNORED_TESTS in the coverage task. These tests were formerly enforced not-executed, but are now ignored.
  • Once consuming branches update their framework pointer, they will rename their IGNORED_TESTS definition to UNCOVERED_TESTS. This will restore the old semantics. The consuming PR in the checklist below do this.

PR checklist

@gilles-peskine-arm gilles-peskine-arm added priority-high High priority - will be reviewed soon size-xs Estimated task size: extra small (a few hours at most) labels Mar 26, 2026
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Outcome analysis tasks can have "ignored" tests. Both coverage and driver
tasks actually don't ignore "ignored" tests: an "ignored" test must fail the
verification if it wasn't ignored.

In preparation for distinguishing between truly ignored tests and tests that
must be uncovered, generalize the test case lookup mechanism.

No intended behavior change for `CoverageTask` and `DriverVSReference`.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
For historical reasons, the "ignored" tests in outcome analysis are not
actually ignored: they must not be covered, otherwise the script complains
about an unnecessary exception. In coverage analysis, rename this behavior
to "uncovered", and have "ignored" tests be actually ignored. In driver test
parity analysis, which is now only done in the 3.6 LTS branch, keep the
historical behavior

Consuming branches are currently defining `IGNORED_TESTS` with the
expectation that the test cases must be uncovered. They will need to rename
their definition to `UNCOVERED_TESTS`.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Copy link
Copy Markdown
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks almost good to me. I only have a few comments. While IGNORED_TESTS is not renamed to UNCOVERED_TESTS in consuming branches, we lose the ability to detect spurious uncovered tests, which seems acceptable.

Comment thread scripts/mbedtls_framework/outcome_analysis.py Outdated
Comment thread scripts/mbedtls_framework/outcome_analysis.py Outdated
Comment thread scripts/mbedtls_framework/outcome_analysis.py
Comment thread scripts/mbedtls_framework/outcome_analysis.py
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members. needs-reviewer This PR needs someone to pick it up for review size-s Estimated task size: small (~2d) and removed size-xs Estimated task size: extra small (a few hours at most) labels Apr 7, 2026
Copy link
Copy Markdown
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@valeriosetti valeriosetti self-requested a review April 8, 2026 08:23
@valeriosetti valeriosetti removed the needs-reviewer This PR needs someone to pick it up for review label Apr 8, 2026
Copy link
Copy Markdown
Contributor

@valeriosetti valeriosetti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have one minor suggestion, but overall LGTM. I will let you decide it it's worth to address it or just merge the PR as is.

ignored = self.ignored_tests.contains(test_suite, test_description)
if ignored:
self.note_ignored_test(results, test_suite, test_description)
continue
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have preferred to add the if ignored condition to the if case below instead of continue here, but that's a minor thing.

@github-project-automation github-project-automation bot moved this from In Development to Has Approval in Roadmap pull requests (new board) Apr 8, 2026
@valeriosetti valeriosetti added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members. labels Apr 8, 2026
@gilles-peskine-arm gilles-peskine-arm merged commit b80f4d5 into Mbed-TLS:main Apr 8, 2026
2 checks passed
@github-project-automation github-project-automation bot moved this from Has Approval to Done in Roadmap pull requests (new board) Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Design and code approved - may be waiting for CI or backports priority-high High priority - will be reviewed soon size-s Estimated task size: small (~2d)

Development

Successfully merging this pull request may close these issues.

3 participants