Closed Bug 1260342 Opened 6 years ago Closed 5 years ago

gtests for libssl internal interfaces

Categories

(NSS :: Test, defect)

defect
Not set
normal

Tracking

(firefox48 affected)

RESOLVED WONTFIX
Tracking Status
firefox48 --- affected

People

(Reporter: jld, Assigned: jld)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1222665 added the build support.  There's an outstanding patch to add some actual tests, which has been somewhat upstaged by TLS 1.3 support, but is probably still worth landing.  It really ought to have been a separate bug from the beginning, and now it is.
Freshly rebased.
Attachment #8735693 - Flags: review?(ttaubert)
Comment on attachment 8735693 [details] [diff] [review]
bug1260342-hg0.diff

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

This looks mostly good to me but I'd like to have ekr as the owner of libssl take a look too and give some input on whether this is the right stuff to test, or the right way.

::: external_tests/common/scoped_ptrs.h
@@ +7,5 @@
>  #ifndef scoped_ptrs_h__
>  #define scoped_ptrs_h__
>  
>  #include "keyhi.h"
> +#include "pk11pub.h"

Why is this needed?
Attachment #8735693 - Flags: review?(ttaubert)
Attachment #8735693 - Flags: review?(ekr)
Attachment #8735693 - Flags: feedback+
Comment on attachment 8735693 [details] [diff] [review]
bug1260342-hg0.diff

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

Please put this up in Rietveld
Attachment #8735693 - Flags: review?(ekr)
(In reply to Eric Rescorla (:ekr) from comment #3)
> Please put this up in Rietveld

Done: https://codereview.appspot.com/279190043/#ps40001
This patch bit-rotted (from bug 1266237 and possibly others).  Is it even worth trying to fix it up or should I just wontfix this?

Specifically: The tests here exist because I asked for ideas of proof-of-concept "internal" tests for libssl, got bug 1222665 comment #3, and tried to turn that into an actual thing.  My understanding is that there are now other tests that depend on using non-exported parts of libssl, so this isn't useful for that, and this doesn't seem very useful for its own sake (especially now that improving test coverage is a lower priority than it previously was).
Flags: needinfo?(ekr)
I think WONTFIX is fine.
Flags: needinfo?(ekr)
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.