Open
Bug 467553
Opened 16 years ago
Updated 1 year ago
Individual NSS test scripts do not run stand-alone, and cannot reuse HOSTDIR
Categories
(NSS :: Test, defect, P2)
NSS
Test
Tracking
(Not tracked)
NEW
3.12.4
People
(Reporter: nelson, Unassigned)
References
Details
Attachments
(3 files)
515 bytes,
patch
|
slavomir.katuscak+mozilla
:
review+
|
Details | Diff | Splinter Review |
2.30 KB,
patch
|
slavomir.katuscak+mozilla
:
review+
|
Details | Diff | Splinter Review |
410 bytes,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
All of NSS's tests scripts should be able to run by themselves. It should not be necessary to run them from within all.sh. It should not be necessary to first define environment variables to get them to run. They should do a useful default test in the absence of any environment variables. Presently, on the trunk, ssl.sh does nothing unless NSS_SSL_TESTS is defined in the environment with a value made up of specific substrings. That's not acceptable. In the absence of that environment variable, ssl.sh should run the "normal_normal" test. This can probably be fixed with a one-line patch to ssl.sh.
Reporter | ||
Comment 1•16 years ago
|
||
It also must have NSS_SSL_RUN defined. That also must have a sensible default value.
Reporter | ||
Comment 2•16 years ago
|
||
Slavo, please review this patch
Attachment #350985 -
Flags: review?(slavomir.katuscak)
Reporter | ||
Comment 3•16 years ago
|
||
I think maybe there is yet another shell variable that must be defined, but I have not yet identified it. When I ran ssl.sh with the above patch, the test ran for a while, maybe 10 minutes, and then stopped. The last lines of log output were: strsclnt -q -p 8443 -d ../ext_client -w nss -2 -c 100 -C :C013 -n ExtendedSSLUs er-ec -u \ DAD.client.comcast.net strsclnt started at Tue Dec 2 09:10:12 PST 2008 strsclnt: 98 cache hits; 1 cache misses, 0 cache not reusable 98 stateless resumes strsclnt: -- SSL: Server Certificate Validated. strsclnt: 0 cache hits; 1 cache misses, 0 cache not reusable 0 stateless resumes strsclnt: PR_Connect returned error -5982: Local Network address is in use. strsclnt completed at Tue Dec 2 09:10:13 PST 2008 Usage: expr expression ssl.sh: #: Stress TLS ECDHE-RSA AES 128 CBC with SHA(session ticket, client auth ) produced a returncode of 0, expected is 0. - PASSED trying to kill selfserv with PID 2052 at Tue Dec 2 09:10:13 PST 2008 kill 2052 selfserv with PID 2052 killed at Tue Dec 2 09:10:13 PST 2008 Notice the Usage message in there. I think one of the many lines that uses the expr command attempted to use an undefined environment variable. :( All of the NSS test scripts need to be fixed so that they define default values for all the environment variables they need.
Updated•16 years ago
|
Attachment #350985 -
Flags: review?(slavomir.katuscak) → review+
Reporter | ||
Comment 4•16 years ago
|
||
Following up on comment 3, I now think that Usage message comes from shell functions html_passed or html_detect_core
Reporter | ||
Comment 5•16 years ago
|
||
OK, the usage message comes from html_passed. The variable MSG_ID is empty. The file named by MSG_ID_FILE contains only a newline (an empty line of text). I don't know how this happens, but it appears to be a separate bug.
Reporter | ||
Comment 6•16 years ago
|
||
Checking in ssl.sh; new revision: 1.96; previous revision: 1.95 Slavo, please check all the other NSS test scripts to be sure they can run by themselves (as opposed to being run from within all.sh), without needing to define environment variables.
Reporter | ||
Comment 7•16 years ago
|
||
Comment on attachment 350985 [details] [diff] [review] This patch works for me (checked in) Patch was committed as revision 1.96 on 2008-12-02 09:58
Attachment #350985 -
Attachment description: This patch works for me → This patch works for me (checked in)
Reporter | ||
Comment 8•16 years ago
|
||
The problem originally reported in this bug was a single example of a broader problem that must be addressed in all of NSS's test scripts. Here are some important properties that all NSS test script are supposed to have, but some presently do not. 1. It should be possible to run each and every one of them all by itself, without all.sh. It should be possible to cd into any directory under nss/tests/ and run the script in that directory, all by itself, without needing to define environment variables to define tests to run or other things first (exception is to define HOST and/or DOMSUF on Windows). If any of those scripts require that some other script be run first, such as running cert.sh to create the cert DBs for testing, Each of those scripts should run the prerequisite test scripts itself, automatically. Note that they should test first, to see if the prerequisite script has already been run in that directory (explained below) and avoid running it again if so. 2. After running any test script, it should be possible to run another test script, reusing the same test directory used by the previous run. E.g. after running (say) nss/tests/dbtests.sh, and having it create a test directory, such as (say) mozilla/tests_results/security/MYHOST.1 it should be possible to define the evironment variable HOSTDIR to contain the absolute path name of that test dir, (e.g. HOSTDIR=/home/nelson/nss_tip/mozilla/tests_results/security/MYHOST.1 ) and then run another of NSS's test scripts, such as (say) nss/tests/merge/merge.sh, and have it reuse the test directory specified in hostdir. This is useful to avoid needless repetition of running the prerequisite scripts. Note that init.sh already does most of the work for this, but there are some problems. In my next comment for this bug, I will explain some of the problems that are preventing the scripts from having these properties, and make some suggestions to fix it. I may even have a patch that fixes these problems in some of the script.
Summary: ssl.sh will not run unless NSS_SSL_TESTS is defined to particular strings → Individual NSS test scripts do not run stand-alone, and cannot reuse HOSTDIR
Reporter | ||
Comment 9•16 years ago
|
||
Here are some of the problems that prevent NSS's individual test scripts from being run (when not run by all.sh) or from re-using exist test directories. 1. Scripts specify /bin/sh on line 1, but use features that require /bin/bash. This works when run from all.sh, because all.sh specifies /bin/bash on line 1, and sources the other scripts rather than running them in subshells. SO, when they are run in all.sh, those scripts are interpreted by /bin/bash. But when run by themselves, they typically fail. An example of this sort of problem: The construct export VARIABLE=value doesn't work in /bin/sh. In /bin/sh, you must use either VARIABLE=value; export VARIABLE or VARIABLE=value export VARIABLE Use of $( and ) I do not object to using these superior constructs, but we need to make sure that the scripts will run with bash if they contain them. 2. cleanup.sh removes files that are needed in order to reuse test dirs. This is simple to fix. Just change the definition of TEMPFILES in init.sh. 3. init.sh creates files with names that end in PIDs, e.g. ../tests.pw.$$ These files cannot be reused in later runs, because the script PID ($$) changes. Simply removing all the .$$ suffixes from the file names created by init.sh seems to solve this problem.
Attachment #351509 -
Flags: review?(slavomir.katuscak)
Updated•16 years ago
|
Attachment #351509 -
Flags: review?(slavomir.katuscak) → review+
Reporter | ||
Comment 10•15 years ago
|
||
Comment on attachment 351509 [details] [diff] [review] patch to fix SOME of the problems in SOME scripts (checked in) Checking in common/init.sh; new revision: 1.70; previous revision: 1.69 Checking in tools/tools.sh; new revision: 1.17; previous revision: 1.16
Attachment #351509 -
Attachment description: patch to fix SOME of the problems in SOME scripts → patch to fix SOME of the problems in SOME scripts (checked in)
Reporter | ||
Comment 11•15 years ago
|
||
I'll leave this bug assigned to Slavo to solve the rest of the issues.
Updated•15 years ago
|
Priority: -- → P2
Target Milestone: 3.12.3 → 3.12.4
Comment 13•15 years ago
|
||
I would fix all the other scripts too, but have only tested this change for now so I am only submitting this for review.
Attachment #371927 -
Flags: review?(nelson)
Reporter | ||
Updated•15 years ago
|
Attachment #371927 -
Flags: review?(nelson) → review+
Comment 14•15 years ago
|
||
Comment on attachment 371927 [details] [diff] [review] Fix for cipher.sh (checked in) Checking in cipher/cipher.sh; /cvsroot/mozilla/security/nss/tests/cipher/cipher.sh,v <-- cipher.sh new revision: 1.16; previous revision: 1.15 done
Attachment #371927 -
Attachment description: Fix for cipher.sh → Fix for cipher.sh (checked in)
Comment 15•2 years ago
|
||
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.
Assignee: slavomir.katuscak+mozilla → nobody
Flags: needinfo?(bbeurdouche)
Updated•2 years ago
|
Severity: normal → S3
Comment 16•1 year ago
|
||
We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.
Flags: needinfo?(bbeurdouche)
You need to log in
before you can comment on or make changes to this bug.
Description
•