-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
tests: use assert in strict assertion mode #10606
Conversation
@@ -38,7 +38,7 @@ describe('DOM', () => { | |||
const el = dom.createElement('div'); | |||
assert.equal(el.localName, 'div'); | |||
assert.equal(el.className, ''); | |||
assert.equal(el.className, el.attributes.length); |
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.
no idea what this was trying to test, so I did my best :)
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.
lol this is hilarious
@@ -414,8 +414,8 @@ describe('DetailsRenderer', () => { | |||
assert.equal(anchorEl.href, 'https://www.example.com/script.js'); | |||
assert.equal(sourceLocationEl.textContent, '/script.js:11:5(www.example.com)'); | |||
assert.equal(sourceLocationEl.getAttribute('data-source-url'), sourceLocation.url); | |||
assert.equal(sourceLocationEl.getAttribute('data-source-line'), sourceLocation.line); | |||
assert.equal(sourceLocationEl.getAttribute('data-source-column'), sourceLocation.column); | |||
assert.equal(sourceLocationEl.getAttribute('data-source-line'), `${sourceLocation.line}`); |
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: you like this better than String()? was the line limit reached if you did that?
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: you like this better than String()? was the line limit reached if you did that?
it does go over the limit on the column
lines, but I also don't have a particular preference between the two
@@ -38,7 +38,7 @@ describe('DOM', () => { | |||
const el = dom.createElement('div'); | |||
assert.equal(el.localName, 'div'); | |||
assert.equal(el.className, ''); | |||
assert.equal(el.className, el.attributes.length); |
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.
lol this is hilarious
Strict assertion mode has been recommended since it was added in Node v9.9.0, and treats all the equality asserts as strict equality assertions. It also gives nicer deep equality diffs.
Not much was caught here, which is good news. A few
null
->undefined
andnumber
->string
coercions, and a couple of old audit scores that weren't updated when all scores went numeric.