Skip to content

Conversation

armcknight
Copy link
Member

@armcknight armcknight commented Jun 18, 2025

The profile UI tests all wind up removing launch config files before they end. But if they abnormally exited before being able to do so, then another UI test would wind up running a launch profile.

This change makes sure there is no on-disk data leakage between UI tests in any way, whether that be launch profiles, envelopes, etc.

Noticed this while investigating #5366, but this doesn't fix the root problem of imbalanced bookkeeping mentioned there.

#skip-changelog

@armcknight armcknight changed the title test: always wipe all data on disk before starting a new UI test test(UI): always wipe all data on disk before starting a new UI test Jun 18, 2025
Copy link
Contributor

github-actions bot commented Jun 18, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1218.54 ms 1240.71 ms 22.17 ms
Size 23.75 KiB 847.95 KiB 824.20 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
c319795 1256.18 ms 1266.87 ms 10.69 ms
c021422 1199.15 ms 1222.20 ms 23.05 ms
94e1968 1238.85 ms 1252.02 ms 13.17 ms
861d361 1227.90 ms 1231.45 ms 3.55 ms
32c4446 1225.00 ms 1231.29 ms 6.29 ms
7b022df 1220.53 ms 1227.56 ms 7.03 ms
25d925a 1236.73 ms 1252.86 ms 16.12 ms
2c603bf 1226.66 ms 1243.06 ms 16.40 ms
18d491a 1229.78 ms 1252.67 ms 22.90 ms
9dbf743 1252.10 ms 1262.10 ms 10.00 ms

App size

Revision Plain With Sentry Diff
c319795 20.76 KiB 431.99 KiB 411.22 KiB
c021422 20.76 KiB 435.64 KiB 414.88 KiB
94e1968 21.58 KiB 614.74 KiB 593.16 KiB
861d361 20.76 KiB 435.65 KiB 414.89 KiB
32c4446 22.84 KiB 403.24 KiB 380.39 KiB
7b022df 22.85 KiB 412.95 KiB 390.10 KiB
25d925a 21.58 KiB 418.82 KiB 397.24 KiB
2c603bf 21.58 KiB 417.88 KiB 396.30 KiB
18d491a 21.58 KiB 544.87 KiB 523.29 KiB
9dbf743 20.76 KiB 434.94 KiB 414.18 KiB

Previous results on branch: armcknight/test/always-wipe-data-for-ui-tests

Startup times

Revision Plain With Sentry Diff
0821583 1231.69 ms 1254.98 ms 23.29 ms

App size

Revision Plain With Sentry Diff
0821583 23.75 KiB 846.99 KiB 823.24 KiB

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can do this for all UI tests, cause some of the app launch profiling test need to run without wiping the data. That's maybe also why the UI tests are failing.

@armcknight
Copy link
Member Author

I think I originally had it in BaseUITest.setUp so that it would only set this for the first launch per test case. For some reason I moved it to newAppSession but you're right, the profiling tests call that method multiple times per case. Moving it to setUp is the right call after all, and I think we should move forward with this.

@armcknight armcknight merged commit 40da768 into main Jun 23, 2025
59 of 61 checks passed
@armcknight armcknight deleted the armcknight/test/always-wipe-data-for-ui-tests branch June 23, 2025 19:17
armcknight added a commit that referenced this pull request Jun 23, 2025
…5436)

* test: always wipe all data on disk before starting a new UI test

* do it in BaseUITest.setUp so it only happens once per test case, on the first launch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants