Closed
Bug 998067
Opened 10 years ago
Closed 10 years ago
Add utility code for making it easier to create GTests based on NSS
Categories
(Core :: Security: PSM, enhancement)
Core
Security: PSM
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: briansmith, Assigned: briansmith)
References
Details
Attachments
(1 file)
5.46 KB,
patch
|
keeler
:
review+
|
Details | Diff | Splinter Review |
Currently in GTest we can write: ASSERT_EQ(SECSuccess, SomeFunctionThatReturnsSECStatus(...)); However, when the assertion fails, it is a PITA to debug without knowing the value of PR_GetError(). Thus, I created ASSERT_SECSuccess() which, when the assertion fails, automatically prints out the value of PR_GetError() in the error message. Similarly, we can already write: ASSERT_EQ(SECFailure, SomeFunctionThatReturnSECStatus(...)); ASSERT_EQ(SEC_SOME_ERROR, PR_GetError()); However, it is more convenient to write and read: ASSERT_SECFailure(SEC_SOME_ERROR, SomeFunctionThatReturnsSECStatus(...)); Also, arguably this is more likely to result in correct tests because when we expect SECFailure we usually expect a specific error code; with two separate ASSERT_EQ calls, it is easy to forget the second one that tests the result of PR_GetError(). The patch to integrate this into the build system will be a separate patch.
Attachment #8408586 -
Flags: review?(dkeeler)
Comment on attachment 8408586 [details] [diff] [review] nssgtest.patch Review of attachment 8408586 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nits addressed. I assume nssgtest.cpp needs to be added to a moz.build file as well. ::: security/pkix/test/gtest/nssgtest.cpp @@ +59,5 @@ > +Pred_SECFailure(const char* expectedExpr, const char* actualExpr, > + PRErrorCode expectedErrorCode, SECStatus actual) > +{ > + if (SECFailure == actual && expectedErrorCode == PR_GetError()) > + return AssertionSuccess(); nit: braces around conditional body ::: security/pkix/test/gtest/nssgtest.h @@ +19,5 @@ > +#define mozilla_pkix__nssgtest_h > + > +#ifdef _MSC_VER > +#pragma warning(push) > +#pragma warning(disable: 4275) What is warning 4275? A comment would be appreciated.
Attachment #8408586 -
Flags: review?(dkeeler) → review+
Assignee | ||
Comment 2•10 years ago
|
||
(In reply to David Keeler (:keeler) from comment #1) > I assume nssgtest.cpp needs to be added to a > moz.build file as well. Yes, done. > nit: braces around conditional body Done. > What is warning 4275? A comment would be appreciated. I removed this #pragma warning stuff. It is only needed when compiling with -W4 which we don't do (yet). Thanks for the review. https://hg.mozilla.org/integration/mozilla-inbound/rev/64ce640f432e
Comment 3•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/64ce640f432e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: mozilla31 → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•