Skip to content
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

Make results / statistics visual testing easier with better data population #24203

Merged
merged 7 commits into from
Jul 13, 2023

Conversation

peppy
Copy link
Sponsor Member

@peppy peppy commented Jul 13, 2023

As I work through making the results screen suck less, this has been very helpful.

In the visual test browser, if two `TestResultsWithPlayer` test cases
are ran consecutively, the second would die on `SoloStatisticsWatcher`
seeing duplicated online score IDs. This surfaced after
6ef39b8, which changed
`TestResultsScreen` to inherit `SoloResultsScreen` rather than
`ResultsScreen`.

This is probably _not_ a very good fix, but I'm trying to be pragmatic
for now. `SoloStatisticsWatcher` should probably not live in
`OsuGameBase`.
@bdach
Copy link
Collaborator

bdach commented Jul 13, 2023

I found that with these changes TestSceneResultsScreen could end up crashing if you happened to run one of the TestResultsWithPlayer test cases consecutively in test browser. It's kind of my fault, too, because it's failing due to high coupling of SoloResultsScreen to SoloStatisticsWatcher.

I fixed it in a crude way in 4b06b6f to unblock dependent PRs, but I may look at fixing properly at some point...

@bdach bdach enabled auto-merge July 13, 2023 17:10
@bdach bdach merged commit 601e3fe into ppy:master Jul 13, 2023
9 of 10 checks passed
@peppy peppy deleted the results-screen-testability branch July 17, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants