Bug 240686 - security/nss: regressions tests require bash
Summary: security/nss: regressions tests require bash
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-gecko (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-19 14:55 UTC by Christian Weisgerber
Modified: 2019-09-24 13:46 UTC (History)
0 users

See Also:
bugzilla: maintainer-feedback? (gecko)


Attachments
Run regression tests with bash (1018 bytes, patch)
2019-09-19 14:55 UTC, Christian Weisgerber
jbeich: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Weisgerber freebsd_committer 2019-09-19 14:55:21 UTC
Created attachment 207626 [details]
Run regression tests with bash

Running security/nss's regression tests reveals a number of shell errors from test scripts that use bash features like the built-in UID variable and more importantly arrays:

[: -ne: unexpected operator
./ssl.sh: 1245: Syntax error: word unexpected (expecting ")")
./ectest.sh: 49: Syntax error: word unexpected (expecting ")")

Running the tests with bash instead shows stunningly different results.
Before (sh):

Passed:             3017
Failed:             0
Failed with core:   0
ASan failures:      0
Unknown status:     0

After (bash):

Passed:             14469
Failed:             0
Failed with core:   0
ASan failures:      0
Unknown status:     48
TinderboxPrint:Unknown: 48

Patch attached. We could change the shebangs of all scripts to /usr/local/bin/bash, but there is no need for this; simply calling the top-level script with bash is enough.
Comment 1 Jan Beich freebsd_committer 2019-09-22 22:58:27 UTC
Comment on attachment 207626 [details]
Run regression tests with bash

Looks OK but see comments below. Bash'isms appear to have been introduced recently. Kinda sad as FreeBSD sh is blazingly fast while tests take a lot of time due to lack of parallelism.

> +TEST_DEPENDS=	bash:shells/bash

For "make test" to install the dependency convert "check regression-test test" (multi-name target) to "do-test". When tinderbox was in use by the package cluster (pointyhat at the time) "regression-test" was used and TEST_DEPENDS was handled by tinderbox itself rather than the ports framework.

> +			${LOCALBASE}/bin/bash ./all.sh

Consistently use either "bash" or "${LOCALBASE}/bin/bash". I'd probably choose the former unless there's a good reason to ignore PATH.
Comment 2 commit-hook freebsd_committer 2019-09-24 13:45:18 UTC
A commit references this bug:

Author: naddy
Date: Tue Sep 24 13:45:09 UTC 2019
New revision: 512715
URL: https://svnweb.freebsd.org/changeset/ports/512715

Log:
  Run regression tests with bash since some test scripts unfortunately
  rely on bash features.

  PR:		240686
  Reviewed by:	jbeich

Changes:
  head/security/nss/Makefile
Comment 3 Christian Weisgerber freebsd_committer 2019-09-24 13:46:01 UTC
Committed with tweaks.