-
-
Notifications
You must be signed in to change notification settings - Fork 337
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
Fix v5 tests #2791
Fix v5 tests #2791
Conversation
Touch events test needs update for the changed behavior
Android (legacy) Performance metrics 🚀
|
Android (new) Performance metrics 🚀
|
This is ready for review. |
iOS (legacy) Performance metrics 🚀
|
@@ -11,4 +11,4 @@ startupTimeTest: | |||
|
|||
binarySizeTest: | |||
diffMin: 200 KiB | |||
diffMax: 400 KiB | |||
diffMax: 600 KiB |
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.
Is there a reason for the increase?
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've tracked it down to this JS SDK upgrade. It seems to make both android and iOS bundles larger, but iOS was closer to the edge before so it broke.
I'm comparing this as the last one where it succeeded #2695 (cd7054e) and this 30307a0 as first where it failed.
It's hard to notice because often the metrics fail because of the start times flakiness. Sometimes both start times and sizes.
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.
@vaind would be fine for you? since you came up with the numbers
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.
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.
@marandaneto @vaind Maybe we can solve the sizes in a new PR/Issue as the v4 with the JS SDK that likely caused this is out.
And this RP is blocking the new v5 alpha that I would like to push ASAP.
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 going back to this I've missed that replay
is part of browser
now.
That clearly explains the difference.
📢 Type of change
📜 Description
jsdom
test environment is missing.💡 Motivation and Context
Failing tests
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled🔮 Next steps
#skip-changelog