-
-
Notifications
You must be signed in to change notification settings - Fork 62
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.
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
Use ES2015 syntax for tests #88
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nice! Just a nit with skip
, but feel free to ignore.
|
||
describe("Parses all of the invalid IDLs to check that they blow up correctly", () => { | ||
const dir = pth.join(__dirname, "invalid/idl"); | ||
const skip = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the skip
should probably be a new Set()
. I'm not sure skip
very useful tho, as all the names are going to be unique, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably an array will be sufficient enough as new Set()
will eventually be written as new Set(["testname1", "testname2"])
.
I'm not sure skip very useful tho, as all the names are going to be unique, right?
I'm sorry but not sure why the uniqueness makes skip
not useful, can you clarify more?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the question is, under what circumstances will there be duplicate file names? (Or have I misunderstood what skip
doing?) If the answer is "never", then the uniqueness check is not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I think skip
is for temporarily disabling a test which cannot be fixed in the short term, or just a test-to-do-later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok to keep it as is. If you are happy with it, I'm happy with it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the uniqueness is even an assumption for skip
as it would accidentally skip multiple tests if names are not unique.
var idl = idls[i], json = jsons[i]; | ||
describe("Parses all of the IDLs to produce the correct ASTs", () => { | ||
const dir = pth.join(__dirname, "syntax/idl"); | ||
const skip = {}; // use if we have a broken test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above, about skip
.
expect(error.message).toEqual(err.message); | ||
expect(error.line).toEqual(err.line); | ||
} | ||
for (let i = 0, n = idls.length; i < n; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit:
for(const idl of idls) {
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed typo in code above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't do it because of errors[i]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, ok
published as webidl2@6.0.1 |
Looking at the tests I thought they should be rewritten in ES2015 before any possible fix here.
Additionally this removes lib-cov references that don't exist.