-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
test(browser-integration): Skip "browsertracing/backgroundtab-pageload" test for firefox #7475
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
Conversation
…d" test for firefox This has been flakey... trace.status is sometimes undefined, lets skip for firefox
size-limit report 📦
|
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.
Hmm so #7442 didn't seem to fix all flakes?
Generally skipping firefox sounds reasonable to me as we've seen multiple times in the past that the flakes occur especially on FF.
Did you run the test by any chance for e.g. 100 times? We recently decided that we want to run integration test changes x100 to better check if flakes are gone. Wrapping the sentryTest
in a good old for loop should do.
sentryTest( | ||
'should finish pageload transaction when the page goes background', | ||
async ({ browserName, getLocalTestPath, page }) => { | ||
// TODO: This is flakey on firefox... trace.status is sometimes undefined |
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 we don't even need the TODO here. IMO it's unlikely that we'll fix this any time soon.
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.
Just got 3 (!) of these errors in a single PR, so let's get rid of this!
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.
Yeah, me too. Let's skip the 100x thing for this one.
@billyvg hope you don't mind but I'm gonna merge this in as it's really a pain atm
This has been flakey... trace.status is sometimes undefined, lets skip for firefox