Increase robustness of run_niscc.sh

NEW
Unassigned

Status

NSS
Test
6 years ago
5 years ago

People

(Reporter: kaie, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Jiri Pospisil suggested several improvements to the run_niscc.sh to make it more robust, let's track this round of enhancements in this bug.
(Reporter)

Updated

6 years ago
Depends on: 767146

Comment 1

6 years ago
Created attachment 646512 [details] [diff] [review]
Proposed patch in one ~20kB piece

Hi,
as promised, here is a patch containing few improvements to the run_niscc.script. The bugzilla doesn't seem to allow me attach multiple files at once, so I'm attaching the big patch to this comment and linking it externally in logical pieces with a brief commentary:

1) http://paste2.org/p/2084652 current test uses hardcoded USE_64 variable set to 1. It's much better to set it accordingly to the current architecture, or just leave it untouched, if the variable already exists.

2) http://paste2.org/p/2084654 that's just a typo. If set, variable VERBOSE shall only be set to "-v", because it is directly used as an argument while running nss tools.

3) http://paste2.org/p/2084655 simplified syntax of redirecting of both stdout and stderr to a file via "&>> FILE" doesn't work in older bash environments. ">> FILE 2>&1" works everywhere.

4) http://paste2.org/p/2084656 when I run the test via an automated script, "gmake objdir_name" output contains debug messages about changing directories, which results in garbage. It can be easily silenced via "-s" option. In this piece of patch is also another detection of architecture, which works only as x86_64 and others. As long as we already have a USE_64 variable set accordingly to the current architecture (see point 1), it's a good idea to use it also here.

5) http://paste2.org/p/2084657 the current conditional statements implemented via logical operators are a little obfuscated and hard to read, so I've rewritten them and commented the first clock, as the three blocks are more or less the same.

6) http://paste2.org/p/2084658 I added a commentary header for the force_crash function in the same style as in other functions of the script. I also added a new function selfserv_wrapper, which is useful for running the server process in the background and restart it when it crashes via segfault. Currently if the server process crashes (which is something this test detects, so it should be prepared for it), then the client processes just hang and never end, so the whole test can never end and never give any results concerning the server side. After the patch, the server process is restarted each time it gives the return value of 139, which indicates a segmentation fault.

7) http://paste2.org/p/2084659 this is just the SSL/TLS part of the script rewritten to incorporate the selfserv_wrapper function proposed in point 6.
Attachment #646512 - Flags: review?(kaie)

Comment 2

6 years ago
Created attachment 656450 [details] [diff] [review]
new small patch, to be applied on top of the old one

I recently found a bug in the proposed patch. The problem is with the part '4)', where I blindly assumed that if we are testing system-installed 64bit nss on a 64bit system, then the appropriate libraries are in '/usr/lib64'. But on some systems (RHEL5.x on s390x and ppc64) those 64bit libraries are located in '/usr/lib' and '/usr/lib64' doesn't exist. So I propose to use '/usr/lib64' only when the folder exists and '/usr/lib' otherwise.
Attachment #656450 - Flags: review?(kaie)

Comment 3

6 years ago
Created attachment 658479 [details] [diff] [review]
new small patch, to be applied on top of the old one

I attach an updated verion of the last patch, because I found out it was faulty.
There were two problems:
I. an old overlooked typo - "USE_64" (string) instead of "$USE_64" (variable)
II. bad syntax of the conditional expression.

Otherwise it fixes the issue described in my last comment.
Attachment #656450 - Attachment is obsolete: true
Attachment #656450 - Flags: review?(kaie)
Attachment #658479 - Flags: review?(kaie)
(Reporter)

Updated

5 years ago
Attachment #646512 - Flags: review?(kaie)
(Reporter)

Updated

5 years ago
Attachment #658479 - Flags: review?(kaie)
You need to log in before you can comment on or make changes to this bug.