Why we have NON-cert tests included in our fwts test?

Bug #1373769 reported by Po-Hsu Lin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Checkbox Provider - Base
Fix Released
High
Daniel Manrique

Bug Description

We have many non-cert test in our fwts test script

Since they won't affect our certificate, why should we test them?
It will generate false-positive result in the fwts report
(e.g. HIGH failures in syntaxcheck test)

maybe we can separate them with a --advanced flag, test them only if that flag exist.

# These are advanced tests that shouldn't affect certification status
NON_CERT_TESTS = ['acpiinfo',
                  'bios_info',
                  'cmosdump',
                  'crs',
                  'crsdump',
                  'csm',
                  'dmicheck',
                  'ebdadump',
                  'fan',
                  'gpedump',
                  'hda_audio',
                  'maxfreq',
                  'maxreadreq',
                  'memmapdump',
                  'microcode',
                  'mpdump',
                  'os2gap',
                  'osilinux',
                  'pcc',
                  'pciirq',
                  'plddump',
                  'pnp',
                  'prsdump',
                  'romdump',
                  'securebootcert',
                  'syntaxcheck',
                  'uefidump',
                  'uefirtmisc',
                  'uefirttime',
                  'uefivarinfo'
                  ]

Related branches

Revision history for this message
Colin Ian King (colin-king) wrote :

Begs the question, why use fwts if you ignore some of the above, such as the uefi* tests and syntaxcheck since these do find legitimate kitten killer bugs

Revision history for this message
Daniel Manrique (roadmr) wrote :

The wrapper script needs to know about those tests because, even if they're not certification blockers, other teams (CE QA for one) use the same script and need those tests to be available. Also, the firmware suite for CDTS (IHV driver testing tool) uses those in an "advanced" suite.

For completeness, here's the list of tests that will block a certificate if failed:

CERT_TESTS = ['acpidump',
              'acpitables',
              'apicedge',
              'apicinstance',
              'aspm',
              'bios32',
              'checksum',
              'cstates',
              'dmar',
              'ebda',
              'fadt',
              'hpet_check',
              'klog',
              'mcfg',
              'method',
              'mpcheck',
              'msr',
              'mtrr',
              'nx',
              'oops',
              'uefirtvariable',
              'version',
              'virt',
              'wmi']

Colin, since you're the fwts expert, your advice and suggestions on which of the NON_CERT_TESTS tests would be good to consider cert blockers would be appreciated :)

Sam, I don't see syntaxcheck being run anywhere as part of certification, and actually to be run it would have to be explicitly included in the whitelist with firmware/fwts_syntaxcheck (which again, I don't see in any of our whitelists). Where did you see this?

Changed in plainbox-provider-checkbox:
status: New → Incomplete
Revision history for this message
Po-Hsu Lin (cypressyew) wrote :

Hello Daniel,
the fwts syntaxcheck will be started when we run the fwts test case.
Bug 1365253 is an example of that kind of failures.

Revision history for this message
Daniel Manrique (roadmr) wrote :

OHHH I see now.

The miscellanea/fwts_test job. That's the one that runs the fwts_test script with *all* automated fwts jobs.

My suggestion would be (to avoid modifying behavior of an existing job):

- Update the script so it supports a --certification switch to run only the tests in the cert list. Otherwise, it should keep running all the tests as it is now.

- Add a new (say, firmware/fwts_certification) job that is basically identical to miscellanea/fwts_test, except that it uses --certification. We also need to add the matching log attachment job.

- Modify the certification whitelists so they run firmware/fwts_certification instead of miscellanea/fwts_test.

Usually we don't modify whitelists for older releases to avoid affecting coverage; however, if we're not considering some tests as certification blockers, it would be harmless to stop running them for certification.

The way I propose to fix this, the miscellanea/fwts_test job remains unchanged, however, and this will let other teams (CE, OEM) continue to run all the tests with no change to their whitelists, and still catch the kitten-killing problems Colin was mentioning. By the time a system hits certification, all these should already be solved so we should be OK(ish) not testing for them at that point.

Changed in plainbox-provider-checkbox:
status: Incomplete → Triaged
importance: Undecided → High
milestone: none → 0.13
Revision history for this message
Daniel Manrique (roadmr) wrote :

Here's the story on how those tests were added.

In early August, we were requested to add a series of new tests for CDTS advanced firmware testing. The added tests are the ones listed in the NON_CERT_TESTS sublist (and the ones in INTERACTIVE_TESTS).

Initially we added everything to the main list of tests, but after some trouble we decided to split them into the 3 categories seen now.

I'm the one who did this and essentially this bug is my fault, because I thought the list was only used for building sub-jobs (firmware/fwts_*), and didn't realize that fwts_test without parameters will run all the tests in TESTS.

I'd like to amend my earlier proposal to:

- Default to running only the CERT tests if fwts_test is called without parameters. This fixes the existing miscellanea/fwts_test job without any further changes.
- Add an --advanced switch to run CERT and NON_CERT (or only NON_CERT). This is not used by any jobs currently but it would give us the possibility of running everything.
- CDTS is not affected as it already uses the existing --list-advanced switch to build per-test jobs.

Revision history for this message
Daniel Manrique (roadmr) wrote :

OK, the --all argument already runs all the tests, so the only thing I did really is to make it so that by default it only runs the tests in the CERT sublist. This effectively restores the old behavior for certification, while maintaining the existing one for CDTS and still giving the possibility of using --all in a future job efinition.

Changed in plainbox-provider-checkbox:
status: Triaged → In Progress
assignee: nobody → Daniel Manrique (roadmr)
Changed in plainbox-provider-checkbox:
milestone: 0.13 → 0.14
Changed in plainbox-provider-checkbox:
status: In Progress → Fix Committed
Changed in plainbox-provider-checkbox:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.