Skip to content

Commit

Permalink
Merge pull request #236 from bdach/fix-crosstalk
Browse files Browse the repository at this point in the history
Fix test failures due to crosstalk via static `AppSettings.SaveReplays`
  • Loading branch information
peppy authored May 23, 2024
2 parents 89fd2a1 + 1f8525a commit 5d52c83
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 24 deletions.
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

0 comments on commit 5d52c83

Please sign in to comment.