Refactor `TestSceneScreenFooter` to test entire `OsuScreen`s by LiquidPL · Pull Request #36718 · ppy/osu

LiquidPL · 2026-02-20T01:44:57Z

Part of the ScreenFooter refactor, which intends to move the footer content handling to OsuScreen. This commit updates the ScreenFooter test to operate on entire OsuScreens, in order to better test the entire flow of pushing a screen, and having it create and add its own content to the footer.

This should be 80-90% identical to the original test case structure wise, just that instead of manually manipulating the footer with SetButtons(), various screens with the appropriate buttons are being moved around the screen stack.

Additionally this adds some more tests handling common use cases, as well as removes TestLoadOverlayAfterFooterIsDisplayed(), since as far as I understand the behaviour described in it doesn't actually happen in production code. From what I can see, Screens instantiate their overlays in load(), and then register them in LoadComplete(). There seems to be no case where a ShearedOverlayContainer is created in the middle of a screen's lifecycle.

Part 1 of the `ScreenFooter` refactor, which intends to move the footer content handling to `OsuScreen`. This commit updates the `ScreenFooter` test to operate on entire `OsuScreen`s, in order to better test the entire flow of pushing a screen, and having it create and add its own content to the footer. Additionally this adds some more tests handling common use cases, as well as removes `TestLoadOverlayAfterFooterIsDisplayed()`, since as far as I understand the behaviour described in it doesn't actually happen in production code. From what I can see, Screens instantiate their overlays in `load()`, and then register them in `LoadComplete()` - there is no case where a `ShearedOverlayContainer` is created in the middle of a screen's lifecycle.

LiquidPL · 2026-02-20T13:45:33Z

Actually I wonder if these should just use OsuGameTestScene, since I just realized that ScreenTestScene doesn't have the same back button logic as OsuGame.handleBackButton() and I'm hesitant to copy that thing across...

peppy · 2026-02-23T07:37:18Z

osu.Game/Tests/Visual/ScreenTestScene.cs

                },
                new PopoverContainer
                {
+                    Depth = -1,


What's this doing?

This is meant to make sure the footer is displayed on top of the screen content, was replicating the layout that it has in OsuGame. I suppose it could use a comment explaining the reasoning.

Hmm, in the actual OsuGame, the footer has its own localised PopoverContainer. May want to match that while we're here.

And yeah, a comment mentioning what this is doing would help, I guess you're trying to display higher than something in the base class but it's not obvious what.

Actually, I just noticed something, why is

osu/osu.Game/Tests/Visual/ScreenTestScene.cs

Line 56 in d139f59

content = new Container { RelativeSizeAxes = Axes.Both },

a thing? If ScreenTestScene is meant to test screens end-to-end, why does it have a Content override? I vaguely remember seeing some suites deriving deriving this class that would just Add() content into the scene instead of testing entire screens.

Digging through the git logs I can see that the override was added here for a reason, but I still feel like something's not right here. Feel free to dismiss this if I'm just missing something here.

Not that I want to fix this now, but I feel like the right course of action here is to change the tests using them and remove this container, rather than moving it out of the footer PopoverContainer.

peppy · 2026-02-23T07:47:14Z

osu.Game/Tests/Visual/ScreenTestScene.cs

-                {
-                    RelativeSizeAxes = Axes.Both,
-                    Child = DialogOverlay = new DialogOverlay()
-                },


I'm guessing this is related to the popover depth change, but also not explained anywhere I can find.

peppy · 2026-02-23T07:49:25Z

osu.Game.Tests/Visual/UserInterface/TestSceneScreenFooter.cs

+            }
+        }
+
+        private partial class TestScreenWithOverlayWithoutFooter : TestScreenWithOverlay


I'm getting really lost with all these derived classes. Hard to build up a mental map of what is going on with these present...

Would it be better if instead there was only one screen class which would have the buttons set in the constructor inside each test case?

I'd say so. Less derived/subclasses is better in all cases.

peppy · 2026-02-23T07:52:09Z

Actually I wonder if these should just use OsuGameTestScene, since I just realized that ScreenTestScene doesn't have the same back button logic as OsuGame.handleBackButton() and I'm hesitant to copy that thing across...

Maybe. If that simplifies things then I might prefer it rather than very custom setups. The only reason not to use OsuGameTestScene is that it has a higher overhead (both for visual and non-visual testing) and comes with caveats like draw visualiser generally not working.

peppy

Left some commentary. Seems mostly fine, just want to try and make sure we simplify this stuff as we refactor. Because it's already a bit hard to deal with in master.

I'm not sure why this is here, but this seems like a reasonable thing to do anyway.

This results in much fewer repetitive screen classes.

🙃

Currently, `ScreenTestScene` would not attempt to check whether a screen has any pending overlays or anything that would block the screen exiting. This commit allows subclasses to override the handler in order to provide logic specific to that test scene. Ideally, this should be refactored to match `OsuGame`, however that is in a need of its own refactor, hence why this temporary measure is added.

LiquidPL · 2026-02-25T21:36:59Z

Pushed an extra commit to allow overriding the back button handler in ScreenTestScene as per https://discord.com/channels/90072389919997952/1458132886946451497/1476243192901931208, since it's needed in a few pending PRs of mine to properly test ShearedOverlayContainer hiding.

Part of the screen footer refactor. Once footer content is being managed by `OsuScreen`, the current tests which simply create the tested overlay and `ScreenFooter` in a container will no longer work. This PR refactors them to use `ScreenTestScene` with the setup being creating a dedicated testing `OsuScreen` which does the bare minimum to create the tested overlay and necessary components (eg. `FooterButtonFreeModsV2` for `TestSceneFreeModsOverlay`). Most of the changes here can be described as `%s/<...>Overlay/screen.Overlay/g`, with some minor touchups as necessary, given that we're now testing a more complete flow which checks more things that were previously not handled by the tests. ## [Move footer to front in ScreenTestScene](f8740e0) Self-explanatory. Without it the footer would show below the actual overlay, breaking tests depending on manual input. For the sake of tests not breaking in CI, both #36718 and this have this included - would prefer the former to be merged first since it was already reviewed there. ## `TestSceneModSelectOverlay` There were a few tests (`TestColumnHidingOnIsValidChange`, `TestColumnHidingOnTextFilterChange`, and `TestHidingOverlayClearsTextSearch`) that would create a custom overlay instance instead of the globally provided one. I've tested both and the tests run fine with the default overlay, so they're now using that instead. ## `TestSceneFreeModSelectOverlay` Updated to use footer v2. --------- Co-authored-by: Dean Herbert <pe@ppy.sh>

pull-request-size bot added the size/XL label Feb 20, 2026

LiquidPL changed the title ~~Rewrite TestSceneScreenFooter to test entire OsuScreens~~ Refactor TestSceneScreenFooter to test entire OsuScreens Feb 20, 2026

LiquidPL added 3 commits February 20, 2026 15:42

Move back button tests to TestSceneScreenFooterNavigation

b8e3060

Remove buttons from screen that shoudn't have them

3164e74

Move footer to front in ScreenTestScene

9fd9790

peppy reviewed Feb 23, 2026

View reviewed changes

peppy self-requested a review February 23, 2026 07:50

peppy reviewed Feb 23, 2026

View reviewed changes

LiquidPL added 4 commits February 23, 2026 15:06

Move content out of footer PopoverContainer

8e98699

I'm not sure why this is here, but this seems like a reasonable thing to do anyway.

Clarify why the footer container's depth is overriden

c5446de

Configure each screen directly in its test case

1bcb25b

This results in much fewer repetitive screen classes.

Remove unnecessary nullable flag

b2cb886

🙃

LiquidPL mentioned this pull request Feb 23, 2026

Migrate sheared overlay tests to ScreenTestScene #36736

Merged

Merge branch 'master' into footer-refactor-tests

fbe4594

peppy self-requested a review March 1, 2026 12:34

peppy approved these changes Mar 1, 2026

View reviewed changes

peppy merged commit b88cba0 into ppy:master Mar 1, 2026
9 checks passed

Uh oh!

Conversation

LiquidPL commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LiquidPL commented Feb 20, 2026

Uh oh!

peppy Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

LiquidPL Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peppy Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

LiquidPL Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peppy Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

peppy Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

LiquidPL Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peppy Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

peppy commented Feb 23, 2026

Uh oh!

peppy left a comment

Choose a reason for hiding this comment

Uh oh!

LiquidPL commented Feb 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

LiquidPL commented Feb 20, 2026 •

edited

Loading

LiquidPL Feb 23, 2026 •

edited

Loading

LiquidPL Feb 23, 2026 •

edited

Loading

LiquidPL Feb 23, 2026 •

edited

Loading