401303 - Eliminate extraneous NSS tests

Status: NEW

(NSS :: Test, defect, P2)

Description

[Julien Pierre]

Any test that doesn't exercise the DB code (except initialization) should not be rerun under thte upgrade DB and shared DB tests passes, as it is a waste of time.

Any test that doesn't exercise PKIX should not be rerun with the PKIX code enabled, as it is waste of time.

This should be done unconditionally, not controlled by environment variables.

Comment 1

[Julien Pierre]

Attachment: Patch to help narrow down which tests not to rerun under PKIX

This patch will cause programs to crash every time LIBPKIX code is involved. Any test that passes with this patch can be eliminated for the PKIX run.

Comment 2

[Julien Pierre]

Attachment: output of all.sh with previous patch (PKIX run only)

Comment 3

[Slavomir Katuscak]

Attachment: Patch to reduce number of cycles x tests combinations.

Comment 4

[Slavomir Katuscak]

List of changes in patch from comment 3:

all.sh:
- updated documentation, explained how script works and which option it uses
- using environment variable NSS_CYCLES to select which cycles to run, and NSS_TESTS which tests to run (if not defined, then there are default values set to run all the tests)
- using NSS_SSL_TESTS and NSS_SSL_RUN to select which tests from ssl.sh to run (will be documented later) - for future I would like to use NSS_ prefix for all variables related to NSS testing
- plan to use also NSS_CERT_TESTS to select which certs to generate in cert.sh (will be added later in next patch)
- every test cycle (standard, pkix, upgrade db, shared db) is processed in it's own function which filters out redundant tests which are not important in this cycle, after tests are done, environment settings are restored 

common/init.sh:
- updated function to create backup file (to restore variables after test cycles)
- contains also correction patch for bug 401877 (will not work correctly without it)

ssl/ssl.sh:
- reorganized to have same structure as other test scripts (init, run tests, cleanup)
- function ssl_run_tests runs tests defined in variable NSS_SSL_TESTS, variable NSS_SSL_RUN limits sub-tests run by ssl_run function
- added support also for client in FIPS mode (bug 401001)

iopr/ssl_iopr.sh:
- added changes to restore variables to previous state after tests are run (caused problems when tests were in different order)

This patch contains many changes, please look through it and let me know if there is anything which is not clear or what you would like to improve.

After this patch is checked in, I plan to add also option to select which cert tests to run (which certs to generate) and also modify Tinderboxes to run all 4 cycles in one run.  

Comment 5

[Julien Pierre]

Comment on attachment 329448 [details] [diff] [review]
Patch to reduce number of cycles x tests combinations.

This looks mostly good, except :

1) please merge the changes to init.sh so they apply to the trunk

2) I don't think the PKIX part of the run should not have the SSL cipher coverage tests duplicated

Comment 6

[Slavomir Katuscak]

Julien, I don't understand what you mean as 'should not have the SSL cipher
coverage tests duplicated'.

You probably mean this part:
echo "${NSS_SSL_TESTS}" | grep "_" > /dev/null
RET=$?
NSS_SSL_TESTS=`echo "${NSS_SSL_TESTS}" | sed -e "s/normal//g" -e "s/bypass//g" -e "s/fips//g" -e "s/_//g"`
[ ${RET} -eq 0 ] && NSS_SSL_TESTS="${NSS_SSL_TESTS} bypass_bypass"

What it does - it detects if there is any server/client test defined in NSS_SSL_TESTS (contains _ character), and if there is any, then remove all combinations of server and clients combinations (sed -e) and use only one - bypass_bypass, which should be fastests. 

There is supposed, that PKIX and default/fips/bypass mode on server/client side are independent features and server/client combinations are already tested in standard cycle, so there is no need to test them again.
Let me know if there is any dependency between those features, so I can remove this part and tests all SSL tests also in PKIX cycle. 

Comment 7

[Julien Pierre]

Slavo,

I executed all.sh with your patch applied. I didn't set specific environment variables except for ECC. There were some SSL coverage tests under the PKIX run in results.html, which I feel there should not have been.

Comment 8

[Slavomir Katuscak]

Attachment: Patch v4. (checked in)

Implemented suggestions from comment 5.

Comment 9

[Slavomir Katuscak]

Checking in all.sh;
/cvsroot/mozilla/security/nss/tests/all.sh,v  <--  all.sh
new revision: 1.52; previous revision: 1.51
done
Checking in common/init.sh;
/cvsroot/mozilla/security/nss/tests/common/init.sh,v  <--  init.sh
new revision: 1.69; previous revision: 1.68
done
Checking in iopr/ssl_iopr.sh;
/cvsroot/mozilla/security/nss/tests/iopr/ssl_iopr.sh,v  <--  ssl_iopr.sh
new revision: 1.6; previous revision: 1.5
done
Checking in ssl/ssl.sh;
/cvsroot/mozilla/security/nss/tests/ssl/ssl.sh,v  <--  ssl.sh
new revision: 1.95; previous revision: 1.94
done

Comment 10

[Slavomir Katuscak]

This bug is partially fixed, but there are still things to do:
1. NSS_SSL_TESTS variable to select which SSL tests to run is already done, we need similar variable NSS_CERT_TESTS to select parts from cert.sh to run (especially if cert.sh is run only as dependency of other script)
2. Upgrade to shared DB run many tests we don't need in every run. This should be limited too.

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, and this bug has priority 'P2'.
:beurdouche, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 13

[Marco Castelluccio [:marco]]

We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.