440617 - cert.sh reports false errors and conceals the cause

Status: RESOLVED FIXED

(NSS :: Test, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch that highlights the problematic script code

The code in cert.sh that tests the creation of certs with extensions 
runs certutil twice for each test; once to create the cert and a second
time to pretty-print the created cert.  The log file shows the first 
run but completely conceals the second one.  An error during the second
run is reported as if it was a failure of the first run.  This is 
completely misleading.

The output of the second run is compared against the content of a file
named certext.txt.  An error for one of the test cases in that file 
causes some correct test runs to be reported as failures.  Sadly, due 
to the concealment of the second run, this failure appears to be a 
failure to generate the cert, when in fact the generation succeeded 
entirely.

The attached patch contains two parts.  One part corrects some errors in 
certext.txt, errors that cause successful tests to be reported as failures.  
The other part changes the cert.sh script to show, rather than conceal, the 
extra test run(s) of certutil, and to make their output visible in the log 
file.  The patch to certext.txt is also attached to bug 436957 and there is 
a review request for it there.  The patch to cert.sh is not intended to be
checked in, but rather serves to highlight the places where important 
information is concealed by the present script.  A better patch needs to 
be written to correct these issues. 

The shell function checkRes runs the certutil propram to pretty print the 
entire certificate once for each line of output that is examined.  It would
be better to run the program once, and then examine that output (in a file)
as many times as necessary.

Comment 1

[Slavomir Katuscak]

I reported patch fixing this within partial fix for bug 439015 (fixing more things at once), please review there.

Comment 2

[Slavomir Katuscak]

Attachment: Patch v2

Fixed problems that Nelson mentioned in bug 439015 comment 3.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 326513 [details] [diff] [review]
Patch v2

This patch is an improvement, but I would suggest some additional
changes.

I think that, each time the shell function cert_extensions_test is
invoked, it should html_failed or html_passed exactly once.
At any point where it fails, it should call html_failed and cert_log
and return 1 (as it does in this patch), but in the event of success
it should call html_passed just once at the very end, rather than 
multiple times as it progresses.

Comment 4

[Slavomir Katuscak]

Attachment: Patch v3.

Implemented changes suggested in comment 3.

Comment 5

[Slavomir Katuscak]

Checking in cert.sh;
/cvsroot/mozilla/security/nss/tests/cert/cert.sh,v  <--  cert.sh
new revision: 1.52; previous revision: 1.51
done

Comment 6

[Slavomir Katuscak]

Caused problems on Windows, reopening and backing out patch.

Checking in cert.sh;
/cvsroot/mozilla/security/nss/tests/cert/cert.sh,v  <--  cert.sh
new revision: 1.53; previous revision: 1.52
done

Comment 7

[Slavomir Katuscak]

After better analysis of results, failure found on Windows was found on build before this patch was applied. Checking back to cvs:

Checking in cert.sh;
/cvsroot/mozilla/security/nss/tests/cert/cert.sh,v  <--  cert.sh
new revision: 1.54; previous revision: 1.53
done