-
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
test(smokehouse): add TTFB smoke test #3310
Conversation
thanks @brendankenny! |
@@ -32,6 +32,10 @@ module.exports = [ | |||
'consistently-interactive': { | |||
score: '>=90', | |||
}, | |||
'time-to-first-byte': { | |||
// Can be flaky, so test float rawValue instead of boolean score |
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.
we can add a ?delay=650
or something to the main URL to force it to fail if we want to check the boolean score?
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.
do we prefer failure? either way we're only checking one execution path
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.
this does check that rawValue
is a number and that it's less than 1000. I could tighten that up based on what passes on Travis, though
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 fine either way just wanted to bring it up since the comment suggests that it was trying to check the boolean but couldn't, at first I did misread this as score < 1000 though 😆
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.
Oh I see, yeah, the delay would push score
firmly over to the fail side so we could test reliably.
I'm ok with the current approach rather than slowing it down (which would make me have to figure out new values for all the other numbers in this file :)
4323ea5
to
92ee62b
Compare
@wardpeet
followup to #2231