Skip to content
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

Enable collect_test_cases.py to work for TF-PSA-Crypto #118

Merged

Conversation

Harry-Ramsey
Copy link
Contributor

@Harry-Ramsey Harry-Ramsey commented Jan 9, 2025

This commit adapts collect_test_cases.py to work for Mbed TLS and TF-PSA-Crypto.

PR checklist

Please add the numbers (or links) of the associated pull requests for consuming branches. You can omit branches where this pull request is not needed.

  • crypto PR Mbed-TLS/TF-PSA-Crypto: #134
  • development PR Mbed-TLS/mbedtls not provided.
  • 3.6 PR Mbed-TLS/mbedtls not provided.

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

@Harry-Ramsey Harry-Ramsey force-pushed the independent-collect-test-cases branch from a958e4d to cb943bc Compare January 17, 2025 09:47
@ronald-cron-arm
Copy link
Contributor

@Harry-Ramsey I think you need to base it on the head of mbedtls-framework. The CI has started to complain.

@@ -142,3 +142,16 @@ def is_mbedtls_3_6() -> bool:
return False
with open(os.path.join(root, 'include', 'mbedtls', 'build_info.h'), 'r') as f:
return re.search(r"#define MBEDTLS_VERSION_NUMBER.*0x0306", f.read()) is not None

def is_mbedtls_4_x() -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this check not really future proof? I mean, I know that 4.x is the near future and we're going to deal with it for a while, but this check is going to fail in >= 5.x. At the same time we don't know yet if tests based on this check will change by that time...
Can't we instead live with only is_mbedtls_3.6 and use not is_mbedtls_3.6 in collect_test_cases.py in place of this new function?

Copy link
Contributor

@ronald-cron-arm ronald-cron-arm Jan 17, 2025

Choose a reason for hiding this comment

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

That's a good point. Thus let's stick to:

if build_tree.looks_like_mbedtls_root(project_root) and \
       not build_tree.is_mbedtls_3_6():

and no need to add is_mbedtls_4_x.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed and rebased, thanks.

@Harry-Ramsey Harry-Ramsey force-pushed the independent-collect-test-cases branch 2 times, most recently from 20d63ce to 246dfa2 Compare January 17, 2025 11:49
@valeriosetti valeriosetti self-requested a review January 17, 2025 13:10
valeriosetti
valeriosetti previously approved these changes Jan 17, 2025
Copy link
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 but there are some trailing spaces.

This commit adapts collect_test_cases.py to work for Mbed TLS and
TF-PSA-Crypto.

Signed-off-by: Harry Ramsey <[email protected]>
@Harry-Ramsey Harry-Ramsey force-pushed the independent-collect-test-cases branch from 246dfa2 to 049270e Compare January 17, 2025 13:31
@valeriosetti valeriosetti self-requested a review January 17, 2025 13:32
Copy link
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

@ronald-cron-arm ronald-cron-arm merged commit 1ead596 into Mbed-TLS:main Jan 17, 2025
2 checks passed
@Harry-Ramsey Harry-Ramsey deleted the independent-collect-test-cases branch January 17, 2025 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants