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.

Sign up for GitHub

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

Jump to bottom

Fix test failures in team versus test #15988

Merged
merged 1 commit into from Dec 8, 2021
Merged

Fix test failures in team versus test #15988

merged 1 commit into from Dec 8, 2021

Conversation

bdach
Copy link
Collaborator

@bdach bdach commented Dec 7, 2021

As seen in https://github.com/ppy/osu/runs/4448261203?check_suite_focus=true

Reproduced locally with single thread mode and following patch applied:

diff --git a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs
index b3ea5bdc4a..cb0baef879 100644
--- a/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs
+++ b/osu.Game/Tests/Visual/Multiplayer/TestMultiplayerClient.cs
@@ -230,6 +230,8 @@ public override async Task ChangeSettings(MultiplayerRoomSettings settings)
             // Server is authoritative for the time being.
             settings.PlaylistItemId = Room.Settings.PlaylistItemId;
 
+            await Task.Delay(1000).ConfigureAwait(false);
+
             await changeQueueMode(settings.QueueMode).ConfigureAwait(false);
 
             await ((IMultiplayerClient)this).SettingsChanged(settings).ConfigureAwait(false);

@frenzibyte frenzibyte mentioned this pull request Dec 8, 2021
Closed
peppy
peppy reviewed Dec 8, 2021
View reviewed changes
osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs
@@ -77,7 +77,7 @@ public void TestCreateWithType()
}
});

AddAssert("room type is team vs", () => client.Room?.Settings.MatchType == MatchType.TeamVersus);
AddUntilStep("room type is team vs", () => client.Room?.Settings.MatchType == MatchType.TeamVersus);
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This one isn't required, but I guess there's no harm?

peppy
peppy reviewed Dec 8, 2021
View reviewed changes
osu.Game.Tests/Visual/Multiplayer/TestSceneTeamVersus.cs
@@ -162,13 +162,13 @@ public void TestChangeTypeViaMatchSettings()
}
});

AddAssert("room type is head to head", () => client.Room?.Settings.MatchType == MatchType.HeadToHead);
AddUntilStep("room type is head to head", () => client.Room?.Settings.MatchType == MatchType.HeadToHead);
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Same with this one, not required.

Copy link
Collaborator Author

@bdach bdach Dec 8, 2021 •
edited

Choose a reason for hiding this comment

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

I just changed all of these asserts across the entire file to until steps, because it was bothering me that it was half-and-half even before I came in.

@peppy peppy merged commit 724d72b into ppy:master Dec 8, 2021
@bdach bdach deleted the fix-team-versus-test-failure branch December 8, 2021 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peppy peppy peppy approved these changes

@frenzibyte frenzibyte frenzibyte approved these changes

Assignees
No one assigned
Labels
size/XS type:reliability type:testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bdach @peppy @frenzibyte