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)

Tracking

(Not tracked)

3.12.4

People

(Reporter: nelson, Unassigned)

References

Details

Attachments

(3 files)

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.
It also must have NSS_SSL_RUN defined.  
That also must have a sensible default value.
Slavo, please review this patch
Attachment #350985 - Flags: review?(slavomir.katuscak)
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.
Attachment #350985 - Flags: review?(slavomir.katuscak) → review+
Following up on comment 3, I now think that Usage message comes from 
shell functions html_passed or html_detect_core
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.
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.
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)
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
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)
Attachment #351509 - Flags: review?(slavomir.katuscak) → review+
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)
I'll leave this bug assigned to Slavo to solve the rest of the issues.
Priority: -- → P2
Target Milestone: 3.12.3 → 3.12.4
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)
Attachment #371927 - Flags: review?(nelson) → review+
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)

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)
Severity: normal → S3

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.

Attachment

General

Created:
Updated:
Size: