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

Fix test failures due to crosstalk via static AppSettings.SaveReplays #236

Merged
merged 1 commit into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 28 additions & 17 deletions osu.Server.Spectator.Tests/ScoreUploaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ public ScoreUploaderTests()
[Fact]
public async Task ScoreDataMergedCorrectly()
{
enableUpload();
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object);
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object)
{
SaveReplays = true
};

await uploader.EnqueueAsync(1, new Score
{
Expand All @@ -81,8 +83,10 @@ public async Task ScoreDataMergedCorrectly()
[Fact]
public async Task ScoreUploads()
{
enableUpload();
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object);
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object)
{
SaveReplays = true
};

await uploader.EnqueueAsync(1, new Score());
await uploadsCompleteAsync(uploader);
Expand All @@ -96,8 +100,10 @@ public async Task ScoreUploads()
[Fact]
public async Task ScoreDoesNotUploadIfDisabled()
{
disableUpload();
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object);
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object)
{
SaveReplays = false
};

await uploader.EnqueueAsync(1, new Score());
await Task.Delay(1000);
Expand All @@ -107,8 +113,10 @@ public async Task ScoreDoesNotUploadIfDisabled()
[Fact]
public async Task ScoreUploadsWithDelayedScoreToken()
{
enableUpload();
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object);
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object)
{
SaveReplays = true
};

// Score with no token.
await uploader.EnqueueAsync(2, new Score());
Expand All @@ -129,8 +137,10 @@ public async Task ScoreUploadsWithDelayedScoreToken()
[Fact]
public async Task TimedOutScoreDoesNotUpload()
{
enableUpload();
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object);
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object)
{
SaveReplays = true
};

uploader.TimeoutInterval = 0;

Expand Down Expand Up @@ -162,8 +172,10 @@ public async Task TimedOutScoreDoesNotUpload()
[Fact]
public async Task FailedScoreHandledGracefully()
{
enableUpload();
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object);
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object)
{
SaveReplays = true
};

bool shouldThrow = true;
int uploadCount = 0;
Expand Down Expand Up @@ -197,9 +209,11 @@ public async Task FailedScoreHandledGracefully()
[Fact]
public async Task TestMassUploads()
{
enableUpload();
AppSettings.ReplayUploaderConcurrency = 4;
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object);
var uploader = new ScoreUploader(loggerFactory.Object, databaseFactory.Object, mockStorage.Object)
{
SaveReplays = true
};

for (int i = 0; i < 1000; ++i)
await uploader.EnqueueAsync(1, new Score());
Expand All @@ -209,9 +223,6 @@ public async Task TestMassUploads()
AppSettings.ReplayUploaderConcurrency = 1;
}

private void enableUpload() => AppSettings.SaveReplays = true;
private void disableUpload() => AppSettings.SaveReplays = false;

private async Task uploadsCompleteAsync(ScoreUploader uploader, int attempts = 5)
{
while (uploader.RemainingUsages > 0)
Expand Down
12 changes: 6 additions & 6 deletions osu.Server.Spectator.Tests/SpectatorHubTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public async Task NewUserConnectsAndStreamsData()
[InlineData(true)]
public async Task ReplayDataIsSaved(bool savingEnabled)
{
AppSettings.SaveReplays = savingEnabled;
scoreUploader.SaveReplays = savingEnabled;

Mock<IHubCallerClients<ISpectatorClient>> mockClients = new Mock<IHubCallerClients<ISpectatorClient>>();
Mock<ISpectatorClient> mockReceiver = new Mock<ISpectatorClient>();
Expand Down Expand Up @@ -171,7 +171,7 @@ await hub.EndPlaySession(new SpectatorState
[Fact]
public async Task ReplaysWithoutAnyHitsAreDiscarded()
{
AppSettings.SaveReplays = true;
scoreUploader.SaveReplays = true;

Mock<IHubCallerClients<ISpectatorClient>> mockClients = new Mock<IHubCallerClients<ISpectatorClient>>();
Mock<ISpectatorClient> mockReceiver = new Mock<ISpectatorClient>();
Expand Down Expand Up @@ -335,7 +335,7 @@ public async Task DisconnectedUserSendsQuitState()
[InlineData(BeatmapOnlineStatus.Loved, true)]
public async Task ScoresAreOnlySavedOnRankedBeatmaps(BeatmapOnlineStatus status, bool saved)
{
AppSettings.SaveReplays = true;
scoreUploader.SaveReplays = true;

Mock<IHubCallerClients<ISpectatorClient>> mockClients = new Mock<IHubCallerClients<ISpectatorClient>>();
Mock<ISpectatorClient> mockReceiver = new Mock<ISpectatorClient>();
Expand Down Expand Up @@ -421,7 +421,7 @@ await hub.EndPlaySession(new SpectatorState
[Fact]
public async Task ScoresHaveAllUserRelatedMetadataFilledOutConsistently()
{
AppSettings.SaveReplays = true;
scoreUploader.SaveReplays = true;

Mock<IHubCallerClients<ISpectatorClient>> mockClients = new Mock<IHubCallerClients<ISpectatorClient>>();
Mock<ISpectatorClient> mockReceiver = new Mock<ISpectatorClient>();
Expand Down Expand Up @@ -484,7 +484,7 @@ await hub.EndPlaySession(new SpectatorState
[Fact]
public async Task FailedScoresAreNotSaved()
{
AppSettings.SaveReplays = true;
scoreUploader.SaveReplays = true;

Mock<IHubCallerClients<ISpectatorClient>> mockClients = new Mock<IHubCallerClients<ISpectatorClient>>();
Mock<ISpectatorClient> mockReceiver = new Mock<ISpectatorClient>();
Expand Down Expand Up @@ -530,7 +530,7 @@ await hub.EndPlaySession(new SpectatorState
[Fact]
public async Task ScoreRankPopulatedCorrectly()
{
AppSettings.SaveReplays = true;
scoreUploader.SaveReplays = true;

Mock<IHubCallerClients<ISpectatorClient>> mockClients = new Mock<IHubCallerClients<ISpectatorClient>>();
Mock<ISpectatorClient> mockReceiver = new Mock<ISpectatorClient>();
Expand Down
4 changes: 3 additions & 1 deletion osu.Server.Spectator/Hubs/ScoreUploader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class ScoreUploader : IEntityStore, IDisposable
/// </summary>
public double TimeoutInterval = 30000;

public bool SaveReplays = AppSettings.SaveReplays;

private const string statsd_prefix = "score_uploads";

private readonly Channel<UploadItem> channel = Channel.CreateUnbounded<UploadItem>();
Expand Down Expand Up @@ -58,7 +60,7 @@ public ScoreUploader(
/// <param name="score">The score.</param>
public async Task EnqueueAsync(long token, Score score)
{
if (!AppSettings.SaveReplays)
if (!SaveReplays)
return;

Interlocked.Increment(ref remainingUsages);
Expand Down