HTML Reporter: HTML reporter enhancements for negative asserts#822
HTML Reporter: HTML reporter enhancements for negative asserts#822platinumazure wants to merge 3 commits into
Conversation
HTML reporter will prepend "NOT" to expected value for notEqual, notStrictEqual, notDeepEqual, and notPropEqual assertions. In addition, notOk assertions are also marked as negative for consistency.
|
I signed the CLA a little bit ago. How do I get the error to go away? And the Travis CI build appears to have failed for reasons I very likely didn't introduce in this pull request... And I'm looking for guidance on how best to test the HTML reporter part itself. To be honest it seems like that code could be refactored (i.e., there would be an HTMLReporter object that knows how to spit out the appropriate HTML given a logging context, and that could be unit tested independent of the actual HTML test page.) but that is a separate issue. |
|
Thank you for the contribution! Regarding the CLA: It looks like you signed twice, the first time the email didn't match your git email. I'm not really sure why the check still fails, though. We currently don't have a way to recheck manually, so the only option is to add another (empty) commit to this PR. Regarding the Travis failure, you're right that its not related to your changes. I haven't seen that particular failure before... As for the actual changes: Adding another argument to the already long @leobalter what do you think? |
|
Could we break that contract in 2.0?
|
|
Kind of. We'd have to deprecate the current method, add a new method in a 1.x release, then remove the deprecated method in 2.0 (same process es Emberjs is following). If we came up with a better name, that effort would be easier to justify. |
|
Could I get some guidance on how to add an empty commit to my PR (to re-trigger the CLA and Travis checks)? Thanks! |
|
|
|
@jzaefferer Thanks, that's done and now I'm just waiting for Travis to run. |
|
@jzaefferer Looks like the CLA is good but the Travis build failed for the same reason as before. Seems like a browserstack issue maybe? |
|
@platinumazure the CI fail was due to a browserstack-runner instability. It's not related to your code.
Agreed. We cannot pass an object with the arguments to push as its first argument is always coerced to a boolean value. That's what we need to deprecate, forcing all push calls to have |
There was a problem hiding this comment.
Do you want me to remove that? I couldn't run the QUnit HTML Reporter suite without that change.
There was a problem hiding this comment.
You don't need to remove that. That was something I didn't notice before because I'm running tests only from the cli (grunt and browserstack).
On Jun 17, 2015, at 8:36 PM, Kevin Partington notifications@github.com wrote:
In test/reporter-html/index.html:
@@ -3,7 +3,7 @@
<title>QUnit HTML Reporter Test Suite</title> - - Do you want me to remove that? I couldn't run the QUnit HTML Reporter suite without that change.—
Reply to this email directly or view it on GitHub.
|
LGTM. |
|
Landed this with a rebase. I've filed #827 for a new push method, or at least a new signature. |
|
PR status is "Closed with unmerged commits". I assume maybe some cherry-picking happened (e.g., my empty commit to re-trigger CLA and Travis was intentionally ignored) but I want to be sure nothing important was missed. @jzaefferer have I got it? If so I can delete my source branch. Thanks! |
|
Yes, I've rebased your commits. You can find a reference to the commit that landed above. |
HTML reporter will prepend "NOT" to expected value for notEqual, notStrictEqual, notDeepEqual, and notPropEqual assertions.
In addition, notOk assertions are also marked as negative for consistency, though the HTML reporter special-cases that assertion.