1385061 - Build NSPR tests with NSS make; Add gyp parameters to build/run NSPR tests

Status: RESOLVED FIXED

(NSS :: Test, enhancement, P3)

Description

[Kai Engert [:KaiE:]]

NSS already treats NSPR as a subordinate code, and builds NSPR.

Because we don't have a separate NSPR project and no separate NSPR CI infrastructure, the NSS project should feel responsible for testing NSPR, too, see bug 1385039.

Before running the NSPR tests, we obviously need to build them.

Can we change the NSS build to also build the NSPR tests?

Comment 1

[Kai Engert [:KaiE:]]

Attachment: 1385061-v1.patch

Comment 2

[Franziskus Kiefer [:franziskus]]

Comment on attachment 8891035 [details] [diff] [review]
1385061-v1.patch

Review of attachment 8891035 [details] [diff] [review]:
-----------------------------------------------------------------

::: coreconf/nspr.sh
@@ +43,2 @@
>      run_verbose make -C "$nspr_dir"
> +    echo "NSPR [3/4] make tests ..."

This should respect the `--disable-tests` flag from build.sh [1], i.e. don't do this if `--disable-tests` is set.

Actually. Given that this takes a considerable amount of time I think I'd actually prefer to have it disabled by default and only enabled when using something like `--enable-nspr-tests` in build.sh.

[1] https://searchfox.org/nss/rev/a853912c3d9856737a2f5cc2f8d31146f094b063/build.sh#91

Comment 3

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Bulk change per wleung.

Comment 4

[Kai Engert [:KaiE:]]

I'll try to follow up

Comment 5

[Kai Engert [:KaiE:]]

Attachment: 1385061-v2.patch

Comment 6

[Kai Engert [:KaiE:]]

Attachment: 1385061-v3.patch

Additional parameter to stop building after NSPR. Helpful for bug 1385039.

Comment 7

[J.C. Jones [:jcj] (he/they)]

Comment on attachment 9091392 [details] [diff] [review]
1385061-v3.patch

Review of attachment 9091392 [details] [diff] [review]:
-----------------------------------------------------------------

Just the one nit. Otherwise r+.

::: help.txt
@@ +42,5 @@
>      --no-zdefs       don't set -Wl,-z,defs
>      --static         create static libraries and use static linking
>      --ct-verif       build with valgrind for ct-verif
>      --nspr           force a rebuild of NSPR
> +    --nspr-tests     when building NSPR also build its tests

ISTM that's "also build and run its tests" every time?

Comment 8

[Kai Engert [:KaiE:]]

Comment on attachment 9091392 [details] [diff] [review]
1385061-v3.patch

You're right. Sorry, I need another revision. I need separate switches for building and running the tests, because building should work, running doesn't yet.

Comment 9

[Kai Engert [:KaiE:]]

Attachment: 1385061-v4.patch

separate options for building and running NSPR tests

Comment 10

[Kai Engert [:KaiE:]]

J.C.: This link compares the latest patch to the one you had already r+ed.
Thanks in advance:
https://bugzilla.mozilla.org/attachment.cgi?oldid=9091392&action=interdiff&newid=9091428&headers=1

Comment 11

[J.C. Jones [:jcj] (he/they)]

Comment on attachment 9091428 [details] [diff] [review]
1385061-v4.patch

Review of attachment 9091428 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good now, r=me

Comment 12

[Kai Engert [:KaiE:]]

https://hg.mozilla.org/projects/nss/rev/8b4a226f7d23c8499e58d890b5c3c9001e4366c8