Closed
Bug 1253910
(ecc-tests)
Opened 8 years ago
Closed 7 years ago
ECC tests
Categories
(NSS :: Test, defect)
NSS
Test
Tracking
(firefox47 affected)
RESOLVED
FIXED
3.29
Tracking | Status | |
---|---|---|
firefox47 | --- | affected |
People
(Reporter: franziskus, Assigned: franziskus)
References
(Blocks 2 open bugs)
Details
Attachments
(2 files, 1 obsolete file)
30.00 KB,
patch
|
KaiE
:
review-
ttaubert
:
review+
|
Details | Diff | Splinter Review |
17.60 KB,
patch
|
Details | Diff | Splinter Review |
There are ecl tests in lib/freebl/ecl/tests that are never run (and that don't compile at the moment). We should rewrite those tests as gtests to get test coverage for ecc code.
Assignee | ||
Updated•8 years ago
|
Blocks: ec-cleanup
Assignee | ||
Updated•8 years ago
|
Alias: ecc-tests
Assignee | ||
Comment 1•8 years ago
|
||
this adds some basic ec tests for prime fields that have been previously disabled. In order to make this work I added a new environment variable FREEBL_TEST that exposes all freebl functionality. This will be useful for future testing of freebl in general (e.g. bug 1225323).
Assignee: nobody → franziskuskiefer
Attachment #8756402 -
Flags: review?(ttaubert)
Attachment #8756402 -
Flags: review?(rrelyea)
Attachment #8756402 -
Flags: review?(kaie)
Assignee | ||
Comment 2•8 years ago
|
||
The tests are now disabled when run through all.sh if FREEBL_TEST is not set.
Attachment #8756402 -
Attachment is obsolete: true
Attachment #8756402 -
Flags: review?(ttaubert)
Attachment #8756402 -
Flags: review?(rrelyea)
Attachment #8756402 -
Flags: review?(kaie)
Attachment #8756430 -
Flags: review?(ttaubert)
Attachment #8756430 -
Flags: review?(rrelyea)
Attachment #8756430 -
Flags: review?(kaie)
Comment 3•8 years ago
|
||
Comment on attachment 8756430 [details] [diff] [review] enable-more-ectests.patch v2 Review of attachment 8756430 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. cmd/ec-tests/manifest.mn looks ok, but I'm not too familiar with this stuff. ::: tests/ec/ec-tests.sh @@ +21,4 @@ > # local shell function to initialize this script > ######################################################################## > > +ec-tests_init() Didn't even know that was allowed as a function name, but maybe change this to ec_tests_init()? Same for the other functions.
Attachment #8756430 -
Flags: review?(ttaubert) → review+
Comment 4•8 years ago
|
||
this is the same patch as before, but ignoring whitespace changes.
Comment 5•8 years ago
|
||
It took me a while to understand the intentions here, thanks for answering my questions on IRC. You intend to use a special build mode of NSS, which exports internal APIs, to allow you to access that internal functionality from special test code. If we want to support such a test mode, I suggest to use a more general build flag for that purpose. Instead of FREEBL_TEST, for example NSS_EXPORT_PRIVATE_APIS ? I tried your patch, with FREEBL_TEST=1, but it didn't build the ec-tests binary. I added it locally, and the build fails with unresolved externals, apparently the variable isn't yet passed down to the scripts and makefiles? Please attach a patch that works. As a general improvement to both your new code, and to the existing ecperf.sh script, I'd like to suggest the following addition: ECPERF_OUT=`ecperf` +echo $ECPERF_OUT ECPERF_OUT=`echo $ECPERF_OUT | grep -i "failed"` When reading the full test output.log, this will make it easier to understand that the tests is indeed executed, and to see the output.
Comment 6•8 years ago
|
||
Comment on attachment 8756430 [details] [diff] [review] enable-more-ectests.patch v2 I think this needs more work.
Attachment #8756430 -
Flags: review?(kaie) → review-
Assignee | ||
Comment 7•7 years ago
|
||
We have fbectest and pk11ectest so these tests are not needed anymore. https://hg.mozilla.org/projects/nss/rev/949bab37f7486b052b166368c7fa3abc4a688414
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.29
You need to log in
before you can comment on or make changes to this bug.
Description
•