Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 17, 2019
Merged

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

merged 2 commits into from
Sep 17, 2019

Conversation

ccli8
Copy link
Contributor

@ccli8 ccli8 commented Sep 16, 2019 •
edited
Loading

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.

@ccli8
Convert TZ target name 'NPSA' to test spec platform name
ad5e94a 
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.
@ccli8 ccli8 mentioned this pull request Sep 16, 2019
Merged
@ciarmcom ciarmcom requested review from urutva and a team September 16, 2019 07:00
@ciarmcom
Copy link
Member

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

@mbed-ci
Copy link

mbed-ci commented Sep 16, 2019

Test run: SUCCESS

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

urutva
urutva suggested changes Sep 16, 2019
View reviewed changes
tools/test_api.py Outdated
@@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

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

@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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

tools/test_api.py Outdated
# (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'):
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove PSA check or keep it (no change)?

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

@ccli8
Remove 'PSA' check from test spec conversion
82f60d9 
This is unnecessary because all TZ targets must follow the naming rule: PLATFORM_[NPSA_]S/NS
@jamesbeyond
Copy link
Contributor

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
Copy link
Contributor

0xc0170 commented Sep 17, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Sep 17, 2019

Test run: SUCCESS

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

@0xc0170 0xc0170 merged commit 50b7529 into ARMmbed:master Sep 17, 2019
@ccli8 ccli8 deleted the nuvoton_conv_tz_test_spec2 branch September 17, 2019 10:04
@adbridge
Copy link
Contributor

@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...

@urutva
Copy link
Contributor

urutva commented Sep 18, 2019

@jamesbeyond Yes, we should update https://os.mbed.com/docs/mbed-os/v5.13/porting/porting-security.html

@ccli8
Copy link
Contributor Author

ccli8 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
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@urutva urutva urutva approved these changes

Assignees
No one assigned
Labels
release-version: 5.14.0-rc3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ccli8 @ciarmcom @mbed-ci @jamesbeyond @0xc0170 @adbridge @urutva