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

Move test helper code to the framework repo #8992

Closed
wants to merge 8 commits into from

Conversation

davidhorstmann-arm
Copy link
Contributor

@davidhorstmann-arm davidhorstmann-arm commented Mar 28, 2024

Move the code in tests/src and tests/include to the framework repository.

Depends on Mbed-TLS/mbedtls-framework#6

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

@davidhorstmann-arm davidhorstmann-arm added needs-ci Needs to pass CI tests priority-high High priority - will be reviewed soon labels Mar 28, 2024
@davidhorstmann-arm davidhorstmann-arm force-pushed the move-test-code-to-framework branch 4 times, most recently from d7f5345 to da24ced Compare April 3, 2024 13:42
@Ryan-Everett-arm Ryan-Everett-arm self-requested a review April 3, 2024 14:02
Copy link
Contributor

@Ryan-Everett-arm Ryan-Everett-arm left a comment

Choose a reason for hiding this comment

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

I noticed a discrepancy in the lines added by Mbed-TLS/mbedtls-framework#6 and the lines removed by this PR. It seems that the following files have been added to framework but are not yet removed from the base directory:

  • tests/include/test/psa_memory_poisoning_wrappers.h
  • tests/include/test/psa_test_wrappers.h
  • tests/src/psa_memory_poisoning_wrappers.c
  • tests/src/psa_test_wrappers.c
  • tests/src/test_memory.c

These are the only files remaining in tests/include/tests/src.

framework Show resolved Hide resolved
@davidhorstmann-arm davidhorstmann-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 labels Apr 3, 2024
@Ryan-Everett-arm Ryan-Everett-arm self-requested a review April 3, 2024 17:03
@davidhorstmann-arm davidhorstmann-arm removed the needs-ci Needs to pass CI tests label Apr 4, 2024
Copy link
Contributor

@Ryan-Everett-arm Ryan-Everett-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!

@Ryan-Everett-arm
Copy link
Contributor

I am also not sure whether this needs a 3.6 backport. The ABI checker hasn't failed here, so perhaps it does.

@davidhorstmann-arm
Copy link
Contributor Author

I am also not sure whether this needs a 3.6 backport. The ABI checker hasn't failed here, so perhaps it does.

Yes, I think this needs a 3.6 backport, I'll make one presently

@davidhorstmann-arm davidhorstmann-arm added the needs-backports Backports are missing or are pending review and approval. label Apr 4, 2024
@davidhorstmann-arm davidhorstmann-arm added needs-preceding-pr Requires another PR to be merged first and removed needs-backports Backports are missing or are pending review and approval. labels Apr 4, 2024
Update them instead to refer to framework/include, where the contents
of tests/include have been moved.

Signed-off-by: David Horstmann <[email protected]>
tests/CMakeLists.txt now refers correctly to framework/include rather
than tests/include.

Signed-off-by: David Horstmann <[email protected]>
This all.sh component specifies a path directly so needs editing to
point into the framework submodule.

Signed-off-by: David Horstmann <[email protected]>
These have now been moved to framework/src and framework/include,
respectively.

Signed-off-by: David Horstmann <[email protected]>
The MBEDTLS_TEST_PATH variable now points into the framework submodule,
along with other ad-hoc paths in the Makefiles.

Signed-off-by: David Horstmann <[email protected]>
These are references that are part of the PSA shared memory testing
work and thus were not previously updated.

Signed-off-by: David Horstmann <[email protected]>
@davidhorstmann-arm davidhorstmann-arm added the needs-ci Needs to pass CI tests label Apr 9, 2024
Copy link
Contributor

@Ryan-Everett-arm Ryan-Everett-arm left a comment

Choose a reason for hiding this comment

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

Windows failure on Internal CI but OpenCI passed. LGTM

@@ -297,7 +297,7 @@ add_subdirectory(library)
add_subdirectory(pkgconfig)

Copy link
Contributor

Choose a reason for hiding this comment

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

Missing: apply code_style.py (and more?) to framework files (Mbed-TLS/mbedtls-framework#7).

@ronald-cron-arm ronald-cron-arm self-requested a review May 17, 2024 06:14
@ronald-cron-arm ronald-cron-arm added needs-work and removed needs-reviewer This PR needs someone to pick it up for review needs-review Every commit must be reviewed by at least two team members, needs-ci Needs to pass CI tests labels May 17, 2024
@davidhorstmann-arm
Copy link
Contributor Author

Superseded by #9409

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-preceding-pr Requires another PR to be merged first needs-work priority-high High priority - will be reviewed soon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants