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

Convert TZ target name 'NPSA' to test spec platform name #11487

Merged
merged 2 commits into from Sep 17, 2019

Conversation

@ccli8
Copy link
Contributor

commented Sep 16, 2019

Description

This PR is continuation of #11467 and to convert TZ target name with suffix NPSA #11288 (comment) to test spec platform name. With this PR, TZ targets are named as follows:

  1. All TZ targets should have name pattern: PLATFORM_[PSA_/NPSA_]S/NS, where:
    1. PLATFORM for test spec platform name
    2. PSA/NPSA for PSA/non-PSA targets. Defaults to PSA target on absent.
    3. S/NS for secure/non-secure targets
  2. Secure target may participate in Greentea, so its name is also truncated here.

Related PRs

Continuation of #11467
Required by #11288 (comment)

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@devran01

Release Notes

Define naming rule for TrustZone targets: 'PLATFORM'_['NPSA_']'S'/'NS', where:

  1. 'PLATFORM' for test spec platform name, which is registered in platform database of mbed-os-tools.
  2. 'NPSA' for non-PSA targets. Defaults to PSA target if absent.
  3. 'S'/'NS' for secure/non-secure targets.
1.  All TZ targets should have name pattern: PLATFORM_[PSA_/NPSA_]S/NS, where:
    (1) 'PLATFORM' for test spec platform name
    (2) 'PSA/NPSA' for PSA/non-PSA targets. Defaults to PSA target on absent.
    (3) 'S'/'NS' for secure/non-secure targets
2. Secure target may participate in Greentea, so its name is also truncated here.
@ciarmcom ciarmcom requested review from devran01 and ARMmbed/mbed-os-maintainers Sep 16, 2019
@ciarmcom

This comment has been minimized.

Copy link
Member

commented Sep 16, 2019

@ccli8, thank you for your changes.
@devran01 @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

@mbed-ci

This comment has been minimized.

Copy link

commented Sep 16, 2019

Test run: SUCCESS

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

@@ -2274,20 +2274,21 @@ def test_spec_from_test_builds(test_builds):
for build in test_builds:
# Convert TZ target name to test spec platform name
#
# 1. All TZ targets should have name pattern: PLATFORM_[PSA_]S/NS, where:
# 1. All TZ targets should have name pattern: PLATFORM_[PSA_/NPSA_]S/NS, where:

This comment has been minimized.

Copy link
@devran01

devran01 Sep 16, 2019

Contributor

@ccli8 Sorry for nitpicking. Having PSA in the target name is not mandatory as it is the default. I'd write this as

  1. All TZ targets should have name pattern: PLATFORM_[NPSA_]S/NS, where:
    (2) 'NPSA' for non-PSA targets. Defaults to PSA target if absent.
    (3) 'S'/'NS' for secure/non-secure targets

This comment has been minimized.

Copy link
@0xc0170

0xc0170 Sep 16, 2019

Member

@ccli8 Once updated, leave a comment, we restart CI

# (3) 'S'/'NS' for secure/non-secure targets
# 2. Secure target may participate in Greentea, so its name is also truncated here.
if Target.get_target(test_builds[build]['platform']).is_TrustZone_target:
if test_builds[build]['platform'].endswith('_NS'):
test_builds[build]['platform'] = test_builds[build]['platform'][:-3]
elif test_builds[build]['platform'].endswith('_S'):
test_builds[build]['platform'] = test_builds[build]['platform'][:-2]

if test_builds[build]['platform'].endswith('_PSA'):

This comment has been minimized.

Copy link
@devran01

devran01 Sep 16, 2019

Contributor

@jamesbeyond Since we do not have any PSA targets that ends with _PSA can we remove this check?

This comment has been minimized.

Copy link
@ccli8

ccli8 Sep 16, 2019

Author Contributor

Remove PSA check or keep it (no change)?

This comment has been minimized.

Copy link
@0xc0170

0xc0170 Sep 16, 2019

Member

Possibly to remote, otherwise not blocking this PR. @ccli8 rather to check the above comment

This comment has been minimized.

Copy link
@ccli8

ccli8 Sep 16, 2019

Author Contributor

@0xc0170 @devran01 @jamesbeyond Removed PSA check from test spec name conversion

@0xc0170 0xc0170 added needs: work and removed needs: review labels Sep 16, 2019
This is unnecessary because all TZ targets must follow the naming rule: PLATFORM_[NPSA_]S/NS
@jamesbeyond

This comment has been minimized.

Copy link
Contributor

commented Sep 16, 2019

Hi @ccli8, apologize for misleading messages earlier, that would caused some back and forth changes. I think mbed os teams really need to have some communication, so we can give consistent messages.
So please, follow what @devran01 suggested, as this is the naming conversions that apply to the PSA team.

@devran01 I think we'll need to have a documentation some where about PSA target naming conversions . do you think it make sense to put it here
https://os.mbed.com/docs/mbed-os/v5.13/porting/porting-security.html

From testing perspective, we just care about removing the suffix of the target, if _PSA never been a valid suffix, we can remove the check.

@0xc0170 0xc0170 added needs: CI and removed needs: work labels Sep 17, 2019
@0xc0170

This comment has been minimized.

Copy link
Member

commented Sep 17, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Sep 17, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Sep 17, 2019
@0xc0170 0xc0170 merged commit 50b7529 into ARMmbed:master Sep 17, 2019
25 checks passed
25 checks passed
continuous-integration/jenkins/pr-head This commit looks good
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(-72 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Success!
Details
travis-ci/docs Success!
Details
travis-ci/doxy-spellcheck Success!
Details
travis-ci/events Success! Runtime is 8630 cycles.
Details
travis-ci/gitattributestest Success!
Details
travis-ci/include_check Success!
Details
travis-ci/licence_check Success!
Details
travis-ci/littlefs Success! Code size is 8464B.
Details
travis-ci/psa-autogen Success!
Details
travis-ci/tools-py2.7 Success!
Details
travis-ci/tools-py3.5 Success!
Details
travis-ci/tools-py3.6 Success!
Details
travis-ci/tools-py3.7 Success!
Details
@ccli8 ccli8 deleted the OpenNuvoton:nuvoton_conv_tz_test_spec2 branch Sep 17, 2019
@adbridge

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2019

@ccli8 Please add a release notes section to this PR covering any additional information that users may need to be aware of regarding these changes...

@devran01

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2019

@ccli8

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

Please add a release notes section to this PR covering any additional information that users may need to be aware of regarding these changes...

@adbridge Added Release Notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.