CMake: Add CMake to mbed-psa greentea tests #14828

hazzlim · 2021-06-24T13:37:33Z

Summary of changes

Changed mbed_greentea_add_test macro to require TEST_SOURCES for all files, removing assumption of main.cpp
Refactored all pre-existing greentea tests to add main.cpp using TEST_SOURCES
Added a CMakeLists.txt for each test suite in Mbed OS PSA.
Moved /pal and /val directories into a separate /test_abstraction_layer directory and combined them into one CMake target, which is linked to by the test suites that require them.

Fixes #14680

Impact of changes

None.

Migration actions required

None.

Documentation

None.

Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[x] Tests / results supplied as part of this PR

Manual testing for pre-existing greentea tests:

All pre-existing tests built successfully using below commands, or fail as expected due to a [NOT_SUPPORTED] error.

Manual testing for PSA tests:
Successfully able to build the test suite with below manual steps:

mbedtools configure -t <toolchain> -m <target> --mbed-os-path <path-to-mbed-os> --app-config TESTS/configs/experimental.json (requires this PR)
cd cmake_build/<target>/develop/<toolchain>/ && cmake ../../../.. -G Ninja -DMBED_TEST_LINK_LIBRARIES=mbed-os && cmake --build .
mbedhtrun -f <path/to/image> -p <port> -d <mount>

All built successfully with both GCC_ARM and ARMC6, and run successfully on target K66F, except:

entropy_inject: can only run with MBEDTLS_ENTROPY_NV_SEED defined; build fails with #error [NOT_SUPPORTED] PSA entropy injection tests can run only with MBEDTLS_ENTROPY_NV_SEED enabled. (Build successful if the macro is defined, but test crashes when run on K66F)
compliance_its/test_s003: can only run with PS_ALLOW_ENTIRE_STORAGE_FILL defined; build fails with #error [NOT_SUPPORTED] Test is too long for CI, thus always fails on timeout. (Build successful if the macro is defined, and test runs to {{result;success}})

Reviewers

@Patater @LDong-Arm

ciarmcom · 2021-06-24T14:00:16Z

@hazzlim, thank you for your changes.
@Patater @LDong-Arm @ARMmbed/mbed-os-core @ARMmbed/mbed-os-security @ARMmbed/mbed-os-maintainers please review.

LDong-Arm

@hazzlim Thanks for the PR, it looks pretty good to me! I will try a few tests locally.

...ATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_MBED_PSA_SRV/TESTS/attestation/test/CMakeLists.txt

...AL_API/FEATURE_PSA/TARGET_MBED_PSA_SRV/TESTS/compliance_attestation/test_a001/CMakeLists.txt

...ATURE_EXPERIMENTAL_API/FEATURE_PSA/TARGET_MBED_PSA_SRV/TESTS/crypto_init/test/CMakeLists.txt

tools/cmake/mbed_greentea.cmake

LDong-Arm

Thanks @hazzlim, it looks almost perfect to me - nice improvement to mbed_greentea_add_test.
Just one remark on a line of comment.

hal/tests/TESTS/mbed_hal/flash/functional_tests/CMakeLists.txt

hal/tests/TESTS/mbed_hal_fpga_ci_test_shield/CMakeLists.txt

tools/cmake/mbed_greentea.cmake

LDong-Arm

LGTM

hal/tests/TESTS/mbed_hal/flash/functional_tests/CMakeLists.txt

hal/tests/TESTS/mbed_hal_fpga_ci_test_shield/CMakeLists.txt

tools/cmake/mbed_greentea.cmake

...PERIMENTAL_API/FEATURE_PSA/TARGET_MBED_PSA_SRV/TESTS/compliance_attestation/test_a001/main.c

0xc0170 · 2021-06-29T11:50:25Z

connectivity/FEATURE_BLE/source/cordio/TESTS/cordio_hci/driver/CMakeLists.txt

@@ -11,6 +11,11 @@ include(${MBED_PATH}/tools/cmake/mbed_greentea.cmake)
 project(${TEST_TARGET})

 mbed_greentea_add_test(
-    TEST_NAME ${TEST_TARGET}
-    TEST_REQUIRED_LIBS mbed-ble mbed-events
+    TEST_NAME


The style change - was it done by a tool or just follow CMake docs style?

Was done to follow style pointers from Lingkai for previous commits

Possibly a bit of an over-interpretation - is it best as it was?

This is the kind of indentation we have when calling a macro function with parameters, so it looks good to me. I guess what @0xc0170 means is whether it was done manually or by some tool, and the former is the case.

We should set-up cmake-format as a pre-commit check and specify the style we want to use. It would be great to run the cmake-format check as part of the CI. That way we will auto-enforce the style and it will always be consistent. We can add a new issue to add cmake-format to the CI in a separate PR.

Pull request has been modified.

hal/tests/TESTS/mbed_hal/flash/functional_tests/CMakeLists.txt

MBED_PATH set to /platform directory incorrectly. Updated to set to /mbed-os directory.

@rajkan01

CMakeLists.txt file in /hal/tests/TESTS/mbed_hal_fpga_ci_test_shield directory was non-functional as it used the greentea_add_test macro, which expects a main.cpp in current directory, but no main.cpp exists there. I checked with @rajkan01 who confirmed that this CMake file is serving no purpose and is there erroneously. All tests in subdirectories of this directory have their own CMakeLists.txt that successfully build them.

The mbed_greentea_add_test macro previously set a variable in order to use the un-prefixed TEST_NAME to refer to the argument in the macro body. Whilst pair-programming with LDong, this was identified and determined to be unecessary (maybe it was a failed attempt to fix something, that was never reversed?) and so it has been removed.

Assumption that greentea test file is always named main.cpp is incorrect. Updated mbed_greentea_add_test() macro to make TEST_SOURCES parameter compulsory, which is used to specify greentea test file(s). This allows tests to use C, or have a different name. Therefore also updated all pre-existing greentea test CMake files to explicity add main.cpp to TEST_SOURCES.

The greentea test for mbed-psa attestation can now be built with CMake.

The greentea test for mbed-psa crypto_init can now be built with CMake.

The greentea test for mbed-psa its_ps can now be built with CMake.

The greentea test for PSA entropy_inject can now be built with CMake. Note: requires MBEDTLS_ENTROPY_NV_SEED enabled, so not tested on target.

Move /val and /pal directories into /test_abstraction_layers directory and combine into one CMake target, mbed-psa-tal. Moved into seperate directory in order to have own CMakeLists.txt, rather than adding to /TARGET_MBED_PSA_SRV CMake file.

Greentea test for PSA compliance_attestation can now build with CMake.

PSA compliance_its tests can now be built with CMake.

Pull request has been modified.

Patater

LGTM

0xc0170 · 2021-06-29T12:56:53Z

CI started

mbed-ci · 2021-06-29T13:42:55Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️
jenkins-ci/mbed-os-ci_tfm-integration	✔️

hazzlim changed the title ~~CMake: Add CMake to Mbed-psa greentea tests~~ CMake: Add CMake to Mbed-psa greentea tests - Closes #14680 Jun 24, 2021

hazzlim changed the title ~~CMake: Add CMake to Mbed-psa greentea tests - Closes #14680~~ CMake: Add CMake to Mbed-psa greentea tests Jun 24, 2021

hazzlim changed the title ~~CMake: Add CMake to Mbed-psa greentea tests~~ CMake: Add CMake to mbed-psa greentea tests Jun 24, 2021

hazzlim marked this pull request as ready for review June 24, 2021 13:42

ciarmcom added the release-type: feature label Jun 24, 2021

ciarmcom requested review from LDong-Arm, Patater and a team June 24, 2021 14:00

ciarmcom requested a review from a team June 24, 2021 14:00

ciarmcom added needs: review component: core component: security component: tools labels Jun 24, 2021

LDong-Arm reviewed Jun 24, 2021

View reviewed changes

LDong-Arm requested a review from a team June 24, 2021 14:22

LDong-Arm linked an issue Jun 24, 2021 that may be closed by this pull request

CMake supports missing for Mbed OS PSA tests #14680

Closed

hazzlim force-pushed the add-cmake-supports-psa-tests branch 2 times, most recently from 341080e to 49e3182 Compare June 28, 2021 13:28

LDong-Arm reviewed Jun 28, 2021

View reviewed changes

hal/tests/TESTS/mbed_hal/flash/functional_tests/CMakeLists.txt Show resolved Hide resolved

hal/tests/TESTS/mbed_hal_fpga_ci_test_shield/CMakeLists.txt Show resolved Hide resolved

tools/cmake/mbed_greentea.cmake Outdated Show resolved Hide resolved

hazzlim force-pushed the add-cmake-supports-psa-tests branch from 49e3182 to 53d9e7f Compare June 28, 2021 14:07

LDong-Arm previously approved these changes Jun 28, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Jun 28, 2021

Patater previously requested changes Jun 29, 2021

View reviewed changes

mergify bot added needs: work and removed needs: CI labels Jun 29, 2021

0xc0170 reviewed Jun 29, 2021

View reviewed changes

hazzlim force-pushed the add-cmake-supports-psa-tests branch from 53d9e7f to b4fdb5d Compare June 29, 2021 12:15

Patater previously requested changes Jun 29, 2021

View reviewed changes

hal/tests/TESTS/mbed_hal/flash/functional_tests/CMakeLists.txt Show resolved Hide resolved

hazzlim added 11 commits June 29, 2021 13:32

CMake: fix incorrect relative path for MBED_PATH

8b020f6

MBED_PATH set to /platform directory incorrectly. Updated to set to /mbed-os directory.

CMake: Add support for PSA attestation test

4bed069

The greentea test for mbed-psa attestation can now be built with CMake.

CMake: Add support for PSA crypto_init test

9dc39a8

The greentea test for mbed-psa crypto_init can now be built with CMake.

CMake: Add support for PSA its_ps test

24a3dab

The greentea test for mbed-psa its_ps can now be built with CMake.

CMake: Add support for PSA entropy_inject test

26290ad

The greentea test for PSA entropy_inject can now be built with CMake. Note: requires MBEDTLS_ENTROPY_NV_SEED enabled, so not tested on target.

CMake: Add support for compliance_attestation test

13ea9f3

Greentea test for PSA compliance_attestation can now build with CMake.

CMake: Add support for PSA compliance_its tests

f69a375

PSA compliance_its tests can now be built with CMake.

hazzlim force-pushed the add-cmake-supports-psa-tests branch from b4fdb5d to f69a375 Compare June 29, 2021 12:34

Patater approved these changes Jun 29, 2021

View reviewed changes

LDong-Arm approved these changes Jun 29, 2021

View reviewed changes

0xc0170 approved these changes Jun 29, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: work labels Jun 29, 2021

mergify bot added ready for merge and removed needs: CI labels Jun 29, 2021

Patater merged commit 472c688 into ARMmbed:master Jun 29, 2021

mergify bot removed the ready for merge label Jun 29, 2021

mbedmain added release-version: 6.13.0 Release-pending and removed release-type: feature Release-pending labels Jul 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CMake: Add CMake to mbed-psa greentea tests #14828

CMake: Add CMake to mbed-psa greentea tests #14828

hazzlim commented Jun 24, 2021 •

edited

Loading

ciarmcom commented Jun 24, 2021

LDong-Arm left a comment

LDong-Arm left a comment

LDong-Arm left a comment

0xc0170 Jun 29, 2021

hazzlim Jun 29, 2021

hazzlim Jun 29, 2021

LDong-Arm Jun 29, 2021

rwalton-arm Jun 29, 2021 •

edited

Loading

Patater left a comment

0xc0170 commented Jun 29, 2021

mbed-ci commented Jun 29, 2021

CMake: Add CMake to mbed-psa greentea tests #14828

CMake: Add CMake to mbed-psa greentea tests #14828

Conversation

hazzlim commented Jun 24, 2021 • edited Loading

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Jun 24, 2021

LDong-Arm left a comment

Choose a reason for hiding this comment

LDong-Arm left a comment

Choose a reason for hiding this comment

LDong-Arm left a comment

Choose a reason for hiding this comment

0xc0170 Jun 29, 2021

Choose a reason for hiding this comment

hazzlim Jun 29, 2021

Choose a reason for hiding this comment

hazzlim Jun 29, 2021

Choose a reason for hiding this comment

LDong-Arm Jun 29, 2021

Choose a reason for hiding this comment

rwalton-arm Jun 29, 2021 • edited Loading

Choose a reason for hiding this comment

Patater left a comment

Choose a reason for hiding this comment

0xc0170 commented Jun 29, 2021

mbed-ci commented Jun 29, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

hazzlim commented Jun 24, 2021 •

edited

Loading

rwalton-arm Jun 29, 2021 •

edited

Loading