Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: More detailed assert message for rule-tester #9769

Merged
merged 4 commits into from Jan 4, 2018
Merged

Update: More detailed assert message for rule-tester #9769

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7252351
Update: More detailed assert message for rule-tester
starkwang Dec 25, 2017
a3f60f0
[squash]add try/finally block
starkwang Dec 26, 2017
2d33618
[squash]append message instead of overwriting it
starkwang Dec 27, 2017
f17d679
[squash]add parentheses surrounding error message
starkwang Dec 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/testers/rule-tester.js
View file
Open in desktop
Expand Up @@ -138,7 +138,14 @@ const IT = Symbol("it");
* @returns {any} Returned value of `method`.
*/
function defaultHandler(text, method) {
return method.apply(this);
try {
return method.apply(this);
} catch (err) {
if (err instanceof assert.AssertionError) {
err.message = `${util.inspect(err.actual)} ${err.operator} ${util.inspect(err.expected)}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this append to the existing message rather than overwriting it? I think it would be good to retain the "Output is incorrect" text to make the source of the error clearer.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I forgot to explain here.
The defaultHandler is called nestedly somehow (sorry I don't dive into the code but its behaviour shows that), so the thrown error will be caught and err message
will be appended in multiple times. It will finally become like

"Output is incorrect 'a' == 'b' 'a' == 'b' 'a' == 'b'"

That's the reason for overwriting it here.

Another solution is adding a flag like appended on error object to avoid multiple appending. But is it worth to do it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. I think that's happening because defaultHandler is used for both the "describe" and the "it" methods. So effectively something like this is happening:

defaultHandler("foo", () => {
    defaultHandler("bar", () => {
        // throw assertion error
    });
});

One solution would be to use the old default handler (which just calls the function in the second argument) as the default for describe, and use the handler that appends a diff as the default for it.

}
throw err;
}
}

class RuleTester {
Expand Down
35 changes: 35 additions & 0 deletions tests/lib/testers/no-test-runners.js
View file
Open in desktop
@@ -0,0 +1,35 @@
/**
* @fileoverview Tests for RuleTester without any test runner
* @author Weijia Wang <starkwang@126.com>
*/
"use strict";

/* global describe, it */
/* eslint-disable no-global-assign*/
const assert = require("assert"),
RuleTester = require("../../../lib/testers/rule-tester");
const tmpIt = it;
const tmpDescribe = describe;

it = null;
describe = null;

try {
const ruleTester = new RuleTester();

assert.throws(() => {
ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
valid: [
"bar = baz;"
],
invalid: [
{ code: "var foo = bar;", output: "invalid output", errors: 1 }
]
});
}, /' foo = bar;' == 'invalid output'/);
} catch (e) {
throw e;
} finally {
it = tmpIt;
describe = tmpDescribe;
}