-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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(font-size): assert on source #9413
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.
feels like we need to do some virtual jumping jacks to atone for our 😈
🤸♂ 🤸♂ 🤸♂ 🤸♂ 🤸♂ 🤸♂ 🤸♂
@@ -280,6 +281,7 @@ describe('SEO: Font size audit', () => { | |||
attributes: ['size', '10px'], | |||
}); | |||
|
|||
expect(auditResult.details.items[0].source).toBe('http://www.example.com'); |
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.
it's also funny because the whole reason I recommended toMatchObject
in the first place was because you were asserting against .source
and .selector
separately 😆
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.
done
@@ -238,7 +238,7 @@ describe('SEO: Font size audit', () => { | |||
expect(auditResult.notApplicable).toBe(true); | |||
}); | |||
|
|||
describe('attributes source location', () => { | |||
describe('attributes style source', () => { | |||
async function runFontSizeAuditWithSingleFailingStyle(style, nodeProperties) { | |||
const artifacts = { | |||
URL: {finalUrl: 'http://www.example.com'}, |
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.
reuse URL
?
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.
done
@@ -238,7 +238,7 @@ describe('SEO: Font size audit', () => { | |||
expect(auditResult.notApplicable).toBe(true); | |||
}); | |||
|
|||
describe('attributes source location', () => { | |||
describe('attributes style source', () => { |
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.
attributes style source
are all nouns so.... wb attributes source of style
? still all nouns but BAM a preposition too
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.
haha love 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.
LGTM? 😬
😆
🤸♂️ 🤸♂️ done |
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.
LGTM, nice job breaking out the PRs into little ones!
cont. #9400