1176732 - Fix for bug 102794 breaks gtests

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Eric Rescorla (:ekr)]

[  FAILED  ] 5 tests, listed below:
[  FAILED  ] SkipTls10/TlsSkipTest.SkipCertificate/0, where GetParam() =
("TLS", 769)
[  FAILED  ] SkipVariants/TlsSkipTest.SkipCertificate/0, where GetParam() =
("TLS", 770)
[  FAILED  ] SkipVariants/TlsSkipTest.SkipCertificate/1, where GetParam() =
("TLS", 771)
[  FAILED  ] SkipVariants/TlsSkipTest.SkipCertificate/2, where GetParam() =
("DTLS", 770)
[  FAILED  ] SkipVariants/TlsSkipTest.SkipCertificate/3, where GetParam() =
("DTLS", 771)

Comment 1

[Kai Engert [:KaiE:]]

In all failed tests, the expected error was 
  SSL_ERROR_RX_UNEXPECTED_HELLO_DONE

but the test failed with
  SSL_ERROR_RX_UNEXPECTED_SERVER_KEY_EXCH

Comment 2

[Eric Rescorla (:ekr)]

(In reply to Kai Engert (:kaie) from comment #1)
> In all failed tests, the expected error was 
>   SSL_ERROR_RX_UNEXPECTED_HELLO_DONE
> 
> but the test failed with
>   SSL_ERROR_RX_UNEXPECTED_SERVER_KEY_EXCH

Yes, the problem is that you (implicitly) changed the default from RSA to DHE so you get a different error.

Did you run the gtests prior to landing?

Comment 3

[Eric Rescorla (:ekr)]

MT, please review patch at:
https://codereview.appspot.com/241640043/

Comment 4

[Kai Engert [:KaiE:]]

(In reply to Eric Rescorla (:ekr) from comment #2)
> Did you run the gtests prior to landing?

Yes, I did, but I might have been confused by the output.

The final part of the output on the console says:

Tests summary:
--------------
Passed:             576
Failed:             0
Failed with core:   1
Unknown status:     0

And the "failed with core" was the reference counting issue that I had discussed.

I had not noticed that were other failures reported further up in the output, I trusted the final trailing lines.

Comment 5

[Eric Rescorla (:ekr)]

ugh. Seems like all.sh could do better.

Comment 6

[Eric Rescorla (:ekr)]

mt, please ignore previous review request. here is the right patch
https://codereview.appspot.com/245490043/

Comment 7

[Eric Rescorla (:ekr)]

r+ from MT on rietveld.

Comment 8

[Eric Rescorla (:ekr)]

Committed as: http://hg.mozilla.org/projects/nss/rev/22bc54abbfc7