Merge ~rickwu4444/plainbox-provider-checkbox:add-fw-diagnosis into plainbox-provider-checkbox:master

[Rick Wu]

Add fwts diagnosis test plan to IOT

Add fwts diagnosis test to IOT due to follow:

1. Based on the UC coverage guide[1], fwts is a part of the cert whitelist and should be added into the test scope.

2. There are 26 out 42 projects[2] were use amd64 cpu. It means over half of projects can do fwts. In the other hand, we have more and more upcoming projects especially we like to work with Intel, it may be a good timing to add this kinds of test for iot.

[1] http://certification-static.canonical.com/docs/UC18Coverage.pdf
[2] https://sites.google.com/canonical.com/ceqa-portal/projects/iot-projects?authuser=0

[Vic Liu (zongminl)]

I found the plan name firmware-ubuntucore a bit ambiguous, I'd like to propose to have:

id: iot-firmware-full
nested_part:
    iot-firmware-automated
    iot-firmware-manual

id: iot-firmware-automated
nested_part:
    iot-fwts

id:iot-firmware-manual

id: iot-fwts
nested_part:
    firmware/fwts_desktop_diagnosis
    firmware/fwts_desktop_diagnosis_results.log.gz

review: Needs Fixing

[Rick Wu (rickwu4444)]

Fixed!
1.add iot-fwts plan for fwts diagnosis test
2.add iot-firmware and nested iot-fwts into it.

[Rick Wu (rickwu4444)]

> I found the plan name firmware-ubuntucore a bit ambiguous, I'd like to propose
> to have:
>
> id: iot-firmware-full
> nested_part:
> iot-firmware-automated
> iot-firmware-manual
>
> id: iot-firmware-automated
> nested_part:
> iot-fwts
>
> id:iot-firmware-manual
>
> id: iot-fwts

> nested_part:
> firmware/fwts_desktop_diagnosis
> firmware/fwts_desktop_diagnosis_results.log.gz

review: Needs Resubmitting

[Rick Wu (rickwu4444)]

> > I found the plan name firmware-ubuntucore a bit ambiguous, I'd like to
> propose
> > to have:
> >
> > id: iot-firmware-full
> > nested_part:
> > iot-firmware-automated
> > iot-firmware-manual
> >
> > id: iot-firmware-automated
> > nested_part:
> > iot-fwts
> >
> > id:iot-firmware-manual
> >
> > id: iot-fwts
>
> > nested_part:
> > firmware/fwts_desktop_diagnosis
> > firmware/fwts_desktop_diagnosis_results.log.gz

Fixed!
1.add iot-fwts plan for fwts diagnosis test
2.add iot-firmware and nested iot-fwts into it.

[Vic Liu (zongminl)]

LGTM, +1

review: Approve

[Kristin Chuang (kristinchuang)]

+1

review: Approve

[Jonathan Cave (jocave)]

I don't follow why there is two sets of test plans one called iot-fwts-* and one called iot-firmware-*

review: Needs Information

[Maciej Kisielewski (kissiel)]

Looks like the MR is stale, either requires fix or an answer. I'm marking it as "Work in progress".

[Rick Wu (rickwu4444)]

> I don't follow why there is two sets of test plans one called iot-fwts-* and
> one called iot-firmware-*

Hi @Jonathan,
Sorry for late reply.
We'd like to make iot-firmware test more modularization and in case there are future tests that go into this group but is not using fwts. Therefore, we separate into iot-fwts-* and iot-firmware-*.

[Maciej Kisielewski (kissiel)]

I think the split could be done once the need arises. Then we'll have a clearer view on what's needed and how to split and name things. For now I'll keep it simple.

[Maciej Kisielewski (kissiel)]

> I think the split could be done once the need arises. Then we'll have a
> clearer view on what's needed and how to split and name things. For now I'll
> keep it simple.

s/I'll/I'd/

[Sylvain Pineau (sylvain-pineau)]

I think running firmware/fwts_desktop_diagnosis which was originally targeting amd64/desktop iso will lead to more questions in the context of iot. worth noting that on arm64, the compatible set of tests is really restricted/minimal.

I'm questioning here the benefit of adding fwts to iot test plans

review: Needs Information

[Rick Wu (rickwu4444)]

> I think running firmware/fwts_desktop_diagnosis which was originally targeting
> amd64/desktop iso will lead to more questions in the context of iot. worth
> noting that on arm64, the compatible set of tests is really
> restricted/minimal.
>
> I'm questioning here the benefit of adding fwts to iot test plans

Hi @Sylvain,
If you please looks into this site[1] which is record our iot project since 2016. There are 26 out 42 projects were use amd64 cpu. It means over half projects can do fwts. In the other hand, we have more and more upcoming projects especially we like to work with Intel, it may be a good timing to add this kinds of test for iot.
[1] https://sites.google.com/canonical.com/ceqa-portal/projects/iot-projects?authuser=0

[Rick Wu (rickwu4444)]

> I think the split could be done once the need arises. Then we'll have a
> clearer view on what's needed and how to split and name things. For now I'll
> keep it simple.

According to @Maciej's suggestion, leave only iot-fwts to keep it simple

review: Needs Resubmitting

[Rick Wu (rickwu4444)]

> > I think running firmware/fwts_desktop_diagnosis which was originally
> targeting
> > amd64/desktop iso will lead to more questions in the context of iot. worth
> > noting that on arm64, the compatible set of tests is really
> > restricted/minimal.
> >
> > I'm questioning here the benefit of adding fwts to iot test plans
>
> Hi @Sylvain,
> If you please looks into this site[1] which is record our iot project since
> 2016. There are 26 out 42 projects were use amd64 cpu. It means over half
> projects can do fwts. In the other hand, we have more and more upcoming
> projects especially we like to work with Intel, it may be a good timing to add
> this kinds of test for iot.
> [1] https://sites.google.com/canonical.com/ceqa-portal/projects/iot-
> projects?authuser=0

And also based on the UC coverage guide[1], fwts is a part of the cert whitelist and should be added into the test scope.
[1] http://certification-static.canonical.com/docs/UC18Coverage.pdf

[Jonathan Cave (jocave)]

@Rick the indication for this to be reviewed again should be to set the top status back to Needs Review. This is not what a comment with "review: Resubmit" is for.

Before you do could you change the commit message to meet the recommendations in the Checkbox Contribution guide.

Thanks

[Rick Wu (rickwu4444)]

> @Rick the indication for this to be reviewed again should be to set the top
> status back to Needs Review. This is not what a comment with "review:
> Resubmit" is for.
>
> Before you do could you change the commit message to meet the recommendations
> in the Checkbox Contribution guide.
>
> Thanks

@Jonathan, Many thanks for reminding. :)

[Jonathan Cave (jocave)]

For me this specific MR is ok to land - the question remaining is Sylvain's about whether these jobs should be run on IoT devices.

[Sylvain Pineau (sylvain-pineau)]

Thanks for the amd64 stats, make senses to bring such test plan to iot.

review: Approve

diff --git a/units/firmware/test-plan.pxu b/units/firmware/test-plan.pxu
index 1765ced..7db724d 100644
--- a/units/firmware/test-plan.pxu
+++ b/units/firmware/test-plan.pxu
@@ -36,3 +36,30 @@ include:
   firmware/tcglog-required-algs-sha256
   firmware/tcglog-require-pe-image-digests
   firmware/tcglog-dump-attachment
+
+
+id: iot-fwts-full
+unit: test plan
+_name: Test fwts diagnosis with iot project
+_description: Test fwts diagnosis with iot project
+include:
+nested_part:
+    iot-fwts-manual
+    iot-fwts-automated
+
+
+id: iot-fwts-manual
+unit: test plan
+_name: Test fwts diagnosis with iot project (manual)
+_description: Test fwts diagnosis with project (manual)
+include:
+
+
+id: iot-fwts-automated
+unit: test plan
+_name: Test fwts diagnosis with iot project (automated)
+_description: Test fwts diagnosis with iot project (automated)
+include:
+    firmware/fwts_desktop_diagnosis
+    firmware/fwts_desktop_diagnosis_results.log.gz
+