-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
MEDIUM: [Tracking] Implement more E2E tests and add new metrics #30265
Comments
👋 |
👋 |
Also Assigning @AndrewGable as he will most likely help us too 🤝 feel free to unassign if you prefer |
Update: There was again a false positive on the e2e test runs. Added this PR trying to minimise those errors: |
Update: First PR for adding tests for typing is up: |
If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results. If a regression has occurred and you are the assigned CM follow the instructions here. If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future. |
Lies, it was a different pr causing the issue |
This issue has not been updated in over 15 days. @AndrewGable, @hannojg, @mountiny, @perunt eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
(Note: we are internally working on a system to make the e2e tests even better, more news on that soon! ™️ ) |
Thanks! |
adding myself here for reviewing the chat switching one! |
Chat switching is merged 🎊 ! @perunt I think we can add "number of renders when we open chat (it should be 1-2)" to the same test as second metric? Also next up would be the search page test! |
Next is to allocate time for this task in our team and move forward! Nothing blocking as far as I can see. We'll pick this up again next week! |
This issue has not been updated in over 15 days. @dangrous, @hannojg, @perunt eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
We were chatting in slack about pointing these E2E tests at staging and using them to help figure out our "high water point" at which the performance degrades critically with an increasing number of reports across various devices. |
@roryabraham I remember we discussed doing a one-off report for those "high water points" here Is this something we should look at? |
I believe @rinej is working on that |
@kirillzyusko lets finish this one! Which tests are currently missing? I see that we are still missing:
I think for the onyx tests we wanted some tests where we don't load the full app, but just run some performance tests with mock data in onyx to catch performance regressions in onyx. |
@hannojg yes, you are right. I think for number of renders we only track amount of re-renders in Composer. Also I think we'll need to revise the approach for tracking number of re-renders, because now we rely on the fact, that if parent was re-rendered, then children that are not wrapped in memo will be re-rendered. But with |
Just as a proof of my words - below I attached a report (after Performance Comparison Report 📊Significant Changes To DurationThere are no entries Meaningless Changes To DurationShow entries
Show details
|
hi friends! just checking in - where are we on this one? let me know if I can help in any way! |
@dangrous I think we are currently (still) trying to fix e2e tests. And only after that we can start to write new e2e tests 👀 |
how are we looking? |
@dangrous Still fixing e2e tests 😅 Recent RN 0.75 broke them 😔 |
Okay, so the tests are stable again, and we are collecting additional metrics. We will add other tests like chat scrolling etc as mentioned in the issue's description soon! |
Yes, create money request flow - a manual would be good. Not sure if its possible to test the one with a scan, you might have to upload a file |
How are things going here? |
cc @kirillzyusko is working on that (please assign him as well). We've overhauled the e2e pipeline to be more robust. We have to fix one failing test, but are otherwise ready to write some more e2e tests this week (finally!!) |
awesome! |
Yes, I should get some time to work on new e2e tests this week! |
The E2E tests are now set up and working again, lets expand on their options and add more flows and metrics:
@Szymon20000 @hannojg
The text was updated successfully, but these errors were encountered: