Skip Bare Metal green tea test for PSA component #11821

rajkan01 · 2019-11-06T11:45:44Z

Description (required)

PSA test framework requires RTOS to run so added MBED_CONF_RTOS_PRESENT to all the test cases.

Summary of change (What the change is for and why)

PSA components get added to green tea test build based on targets.json.
PSA test framework uses RTOS threads and test cases run on this created threads.
Added MBED_CONF_RTOS_PRESENT guard to all PSA test cases to skip in the green tea test framework

Note: These changes are done as identified that PSA component uses RTOS API and not a very detailed analysis of the component.

Tested the green tea test on below list of targets with bare metal config and added the logs in
IOTCORE-1397[https://jira.arm.com/browse/IOTCORE-1397] Jira ticket

K64F
K66F
Nucleo F429ZI
Nucleo F411RE
Nucleo L073RZ
Nucleo F207ZG

A main green tea test framework and mbed-os test cases change for bare metal is available in PR Bare metal greentea support #11721

Documentation (Details of any document updates required)

Pull request type (required)

[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 (required)

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

Reviewers (optional)

@Patater @jamesbeyond @evedon @hugueskamba

Release Notes (required for feature/major PRs)

Summary of changes

Impact of changes

Migration actions required

-PSA test framework uses the RTOS threads to run test cases so added MBED_CONF_RTOS_PRESENT to all the test cases.

ciarmcom · 2019-11-06T12:00:32Z

@rajkan01, thank you for your changes.
@Patater @evedon @jamesbeyond @hugueskamba @ARMmbed/mbed-os-maintainers @ARMmbed/mbed-os-test @ARMmbed/mbed-os-tls please review.

Patater · 2019-11-06T15:39:57Z

features/frameworks/TARGET_PSA/pal/pal_mbed_os_intf.cpp

    MBED_ASSERT((type > COMPLIANCE_TEST_START) && (type < COMPLIANCE_TEST_END));
    Thread thread(osPriorityNormal, TEST_STACK_SIZE, NULL);
    thread.start(main_wrapper);
    thread.join();
+#endif


Do we need these ifdefs? Will this function run when there is no RTOS? What do we want this function to do if there is no RTOS?

If this function will run, and we want it to be more than a no-op, perhaps we should call main_wrapper() here.

@Patater When we do "mbed test -m NUCLEO_F411RE -t GCC_ARM --app-config TESTS/configs/baremetal.json" then build tools build mbed-os and green tea test with bare-metal configuration and No RTOS.

pal_mbed_os_intf.cpp uses RTOS API which leads to undefined threads APIs and builds fails.so I added the ifdefs to make the build pass.

This is the first level changes to make green tea test to run in bare metal once this PR gets merged then please do necessary changes on PSA to make it work for bare-metal.

Why is pal_mbed_os_intf.cpp included in the build at all? Is it needed for anything in baremetal?

I am happy with this approach (no-op in test_start()) so long as greentea sees the PSA tests are skipped, not that they passed (because pass is meaningless if the tests did nothing).

You could have returned a non-zero value but it is not checked anywhere...

Patater · 2019-11-06T16:46:41Z

features/frameworks/TARGET_PSA/pal/pal_mbed_os_intf.cpp

    MBED_ASSERT((type > COMPLIANCE_TEST_START) && (type < COMPLIANCE_TEST_END));
    Thread thread(osPriorityNormal, TEST_STACK_SIZE, NULL);
    thread.start(main_wrapper);
    thread.join();
+#endif


I am happy with this approach (no-op in test_start()) so long as greentea sees the PSA tests are skipped, not that they passed (because pass is meaningless if the tests did nothing).

Patater · 2019-11-06T16:49:57Z

Please make sure this doesn't break testing on NXP LPC55S69 or Cypress PSoC 6 target. Thanks.

jainvikas8 · 2019-11-07T10:32:16Z

TESTS/psa/its_ps/main.cpp

-#ifndef TARGET_PSA
-#error [NOT_SUPPORTED] ITS/PS tests can run only on PSA-enabled targets.
+#if !defined(TARGET_PSA) || !defined(MBED_CONF_RTOS_PRESENT)
+#error [NOT_SUPPORTED] ITS/PS tests can run only on PSA-enabled targets and RTOS.


PSA-targets with RTOS present.

Yes PSA test cases use threads to run the test case and in bare-metal we don't have any thread support so i guarded with MBED_CONF_RTOS_PRESENT

jainvikas8 · 2019-11-07T10:32:54Z

TESTS/psa/its_ps/main.cpp

@@ -238,4 +238,4 @@ int main()
    return !Harness::run(specification);
 }

-#endif // TARGET_PSA
+#endif // TARGET_PSA || !defined(MBED_CONF_RTOS_PRESENT)


!defined (TARGET_PSA)

jainvikas8 · 2019-11-07T10:35:02Z

components/TARGET_PSA/TESTS/compliance_its/test_s001/main.c

@@ -15,3 +19,4 @@ int main(void)
    test_start(test_entry_p001, COMPLIANCE_TEST_STORAGE);
 #endif
 }
+#endif


Comment update, please. - // !defined(MBED_CONF_RTOS_PRESENT)

jainvikas8 · 2019-11-07T10:35:13Z

components/TARGET_PSA/TESTS/compliance_its/test_s002/main.c

@@ -16,3 +19,4 @@ int main(void)
    test_start(test_entry_p002, COMPLIANCE_TEST_STORAGE);
 #endif
 }
+#endif


jainvikas8 · 2019-11-07T10:35:59Z

components/TARGET_PSA/TESTS/compliance_its/test_s005/main.c

@@ -15,3 +19,4 @@ int main(void)
    test_start(test_entry_p005, COMPLIANCE_TEST_STORAGE);
 #endif
 }
+#endif


Comment update please to all files - // !defined(MBED_CONF_RTOS_PRESENT)

jainvikas8 · 2019-11-07T11:04:14Z

Hi @rajkan01,
So with your changes, I happen to test the LPC55S69 target.
I see PSA compliance test passing with the following commands with sets to default profile.
mbed test --compile -m LPC55S69_NS -n *compliance* -t ARMC6 --run
But if the bare-metal profile is used then I do see them passing rather than skipping, Is this a known behavior?
mbed test -m LPC55S69_NS -t ARMC6 --app-config TESTS/configs/baremetal.json -n *compliance* --run

Please, could you share the mbed test command lines to test this target. Thanks.
Thanks.

evedon · 2019-11-08T09:49:51Z

Hi @rajkan01,
So with your changes, I happen to test the LPC55S69 target.
I see PSA compliance test passing with the following commands with sets to default profile.
mbed test --compile -m LPC55S69_NS -n *compliance* -t ARMC6 --run
But if the bare-metal profile is used then I do see them passing rather than skipping, Is this a known behavior?
mbed test -m LPC55S69_NS -t ARMC6 --app-config TESTS/configs/baremetal.json -n *compliance* --run

Please, could you share the mbed test command lines to test this target. Thanks.
Thanks.

The expected behaviour is that the tests should be skipped. @rajkan01 please look into it

TESTS/psa/its_ps/main.cpp

evedon · 2019-11-08T14:25:59Z

Hi @rajkan01,
So with your changes, I happen to test the LPC55S69 target.
I see PSA compliance test passing with the following commands with sets to default profile.
mbed test --compile -m LPC55S69_NS -n *compliance* -t ARMC6 --run
But if the bare-metal profile is used then I do see them passing rather than skipping, Is this a known behavior?
mbed test -m LPC55S69_NS -t ARMC6 --app-config TESTS/configs/baremetal.json -n *compliance* --run
Please, could you share the mbed test command lines to test this target. Thanks.
Thanks.

The expected behaviour is that the tests should be skipped. @rajkan01 please look into it

@jainvikas8 This PR disables bare metal tests for PSA component but bare metal support in greentea is added in #11721. So if you want to run bare metal tests on LPC55S69 you need both PRs.
Furthermore, I am not sure how you ran the bare metal tests on LPC55S69. @rajkan01 tried to compile the tests for this target and the compilation fails.
We have manually tested this PR on 6 golden boards and have agreed with @jamesbeyond that the rest of the golden boards will be tested in nightly CI and any issue will be handled as defect.

jainvikas8 · 2019-11-08T16:33:28Z

@evedon @rajkan01
I ran those tests following the steps below on the changes of this PR:
Step 1: Delete everything in BUILD directory
Step 2: mbed test --compile -m LPC55S69_NS -n *compliance* -t ARMC6 --run
Step 3: mbed test -m LPC55S69_NS -t ARMC6 --app-config TESTS/configs/baremetal.json -n *compliance* --run

If the above steps are correct:

Realized that I didn't build those tests on step 3 (missing --compile option), of course, it fails if I use that.
Probably step 3 using already compiled tests from step 2?

Run output attached
step2.txt
step3.txt

jamesbeyond · 2019-11-08T16:55:56Z

@evedon @rajkan01
I ran those tests following the steps below on the changes of this PR:
Step 1: Delete everything in BUILD directory
Step 2: mbed test --compile -m LPC55S69_NS -n *compliance* -t ARMC6 --run
Step 3: mbed test -m LPC55S69_NS -t ARMC6 --app-config TESTS/configs/baremetal.json -n *compliance* --run

If the above steps are correct:

Realized that I didn't build those tests on step 3 (missing --compile option), of course, it fails if I use that.

Probably step 3 using already compiled tests from step 2?

Run output attached
step2.txt
step3.txt

you need to remove --run but add -c to give a clean build

jamesbeyond

look good

rajkan01 · 2019-11-11T13:12:49Z

@adbridge @0xc0170 Please start the CI

0xc0170 · 2019-11-11T14:57:37Z

CI started

mbed-ci · 2019-11-11T17:20:46Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2019-11-11T17:25:11Z

Might be internet internal issue, restarted tests

Enable the Bare Metal green tea test

f30dd1c

-PSA test framework uses the RTOS threads to run test cases so added MBED_CONF_RTOS_PRESENT to all the test cases.

ciarmcom requested review from evedon, hugueskamba, jamesbeyond, Patater and a team November 6, 2019 12:00

ciarmcom added the needs: review label Nov 6, 2019

rajkan01 changed the title ~~Enable the Bare Metal green tea test~~ Skip Bare Metal green tea test for PSA component Nov 6, 2019

Patater requested a review from jainvikas8 November 6, 2019 15:37

Patater suggested changes Nov 6, 2019

View reviewed changes

Patater approved these changes Nov 6, 2019

View reviewed changes

jainvikas8 reviewed Nov 7, 2019

View reviewed changes

jamesbeyond suggested changes Nov 8, 2019

View reviewed changes

TESTS/psa/its_ps/main.cpp Outdated Show resolved Hide resolved

Incorporated review comments

829e90a

jamesbeyond approved these changes Nov 8, 2019

View reviewed changes

jainvikas8 approved these changes Nov 11, 2019

View reviewed changes

adbridge added needs: CI and removed needs: review labels Nov 11, 2019

0xc0170 added the release-version: 5.15.0-rc1 label Nov 12, 2019

0xc0170 approved these changes Nov 12, 2019

View reviewed changes

0xc0170 merged commit d556bf6 into ARMmbed:master Nov 12, 2019

0xc0170 removed the needs: CI label Nov 12, 2019

Skip Bare Metal green tea test for PSA component #11821

Skip Bare Metal green tea test for PSA component #11821

Uh oh!

Conversation

rajkan01 commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (required)

Summary of change (What the change is for and why)

Documentation (Details of any document updates required)

Pull request type (required)

Test results (required)

Reviewers (optional)

Release Notes (required for feature/major PRs)

Summary of changes

Impact of changes

Migration actions required

Uh oh!

ciarmcom commented Nov 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajkan01 Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Patater commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajkan01 Nov 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jainvikas8 commented Nov 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evedon commented Nov 8, 2019

Uh oh!

Uh oh!

evedon commented Nov 8, 2019

Uh oh!

jainvikas8 commented Nov 8, 2019

Uh oh!

jamesbeyond commented Nov 8, 2019

Uh oh!

jamesbeyond left a comment

Choose a reason for hiding this comment

Uh oh!

rajkan01 commented Nov 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Nov 11, 2019

Uh oh!

mbed-ci commented Nov 11, 2019

Uh oh!

0xc0170 commented Nov 11, 2019

Uh oh!

Uh oh!

rajkan01 commented Nov 6, 2019 •

edited

Loading

rajkan01 Nov 6, 2019 •

edited

Loading

Patater commented Nov 6, 2019 •

edited

Loading

rajkan01 Nov 8, 2019 •

edited

Loading

jainvikas8 commented Nov 7, 2019 •

edited

Loading

rajkan01 commented Nov 11, 2019 •

edited

Loading