Fix entity leak in SaveLoadSaveTest, StackTest cleanup (#45079)

* StackTest await consolidation, clean SaveLoadSave

* you know what they say about assumptions
This commit is contained in:
Whatstone 2026-08-16 09:40:04 +00:00 • committed by GitHub
parent fc129e6d72
commit f32bc056ae
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 72 additions and 44 deletions

View file

@ -14,7 +14,7 @@ using Robust.Shared.Utility;
namespace Content.IntegrationTests.Tests
{
/// <summary>
/// Tests that a grid's yaml does not change when saved consecutively.
/// Tests that a grid's yaml does not change when saved consecutively.
/// </summary>
[TestFixture]
public sealed partial class SaveLoadSaveTest : GameTest
@ -28,26 +28,30 @@ namespace Content.IntegrationTests.Tests
var mapLoader = entManager.System<MapLoaderSystem>();
var mapSystem = entManager.System<SharedMapSystem>();
var cfg = server.ResolveDependency<IConfigurationManager>();
Assert.That(cfg.GetCVar(CCVars.GridFill), Is.False);
Assume.That(cfg.GetCVar(CCVars.GridFill), Is.False);
var testSystem = server.System<SaveLoadSaveTestSystem>();
testSystem.Enabled = true;
Assume.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the start of CreateSaveLoadSaveGrid");
var rp1 = new ResPath("/save load save 1.yml");
var rp2 = new ResPath("/save load save 2.yml");
MapId mapId0 = MapId.Nullspace;
MapId mapId1 = MapId.Nullspace;
await server.WaitPost(() =>
{
mapSystem.CreateMap(out var mapId0);
mapSystem.CreateMap(out mapId0);
var grid0 = mapSystem.CreateGridEntity(mapId0);
entManager.RunMapInit(grid0.Owner, entManager.GetComponent<MetaDataComponent>(grid0));
Assert.That(mapLoader.TrySaveGrid(grid0.Owner, rp1));
mapSystem.CreateMap(out var mapId1);
mapSystem.CreateMap(out mapId1);
Assert.That(mapLoader.TryLoadGrid(mapId1, rp1, out var grid1));
Assert.That(mapLoader.TrySaveGrid(grid1!.Value, rp2));
});
await server.WaitIdleAsync();
var userData = server.ResolveDependency<IResourceManager>().UserData;
string one;
@ -85,12 +89,18 @@ namespace Content.IntegrationTests.Tests
}
});
testSystem.Enabled = false;
await server.WaitPost(() =>
{
mapSystem.DeleteMap(mapId0);
mapSystem.DeleteMap(mapId1);
});
Assert.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the end of CreateSaveLoadSaveGrid");
}
private new const string TestMap = "Maps/bagel.yml";
/// <summary>
/// Loads the default map, runs it for 5 ticks, then assert that it did not change.
/// Loads the default map, runs it for 5 ticks, then assert that it did not change.
/// </summary>
[Test]
public async Task LoadSaveTicksSaveBagel()
@ -102,6 +112,8 @@ namespace Content.IntegrationTests.Tests
var testSystem = server.System<SaveLoadSaveTestSystem>();
testSystem.Enabled = true;
Assume.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the start of LoadSaveTicksSaveBagel");
var rp1 = new ResPath("/load save ticks save 1.yml");
var rp2 = new ResPath("/load save ticks save 2.yml");
@ -110,23 +122,22 @@ namespace Content.IntegrationTests.Tests
Assert.That(cfg.GetCVar(CCVars.GridFill), Is.False);
// Load bagel.yml as uninitialized map, and save it to ensure it's up to date.
server.Post(() =>
await server.WaitPost(() =>
{
var path = new ResPath(TestMap);
Assert.That(mapLoader.TryLoadMap(path, out var map, out _), $"Failed to load test map {TestMap}");
mapId = map!.Value.Comp.MapId;
Assert.That(mapLoader.TrySaveMap(mapId, rp1));
});
// Run 5 ticks.
server.RunTicks(5);
// Run 5 ticks.
server.RunTicks(5);
});
await server.WaitPost(() =>
{
Assert.That(mapLoader.TrySaveMap(mapId, rp2));
});
await server.WaitIdleAsync();
var userData = server.ResolveDependency<IResourceManager>().UserData;
string one;
@ -166,17 +177,18 @@ namespace Content.IntegrationTests.Tests
testSystem.Enabled = false;
await server.WaitPost(() => mapSys.DeleteMap(mapId));
Assert.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the end of LoadSaveTicksSaveBagel");
}
/// <summary>
/// Loads the same uninitialized map at slightly different times, and then checks that they are the same
/// when getting saved.
/// Loads the same uninitialized map at slightly different times, and then checks that they are the same
/// when getting saved.
/// </summary>
/// <remarks>
/// Should ensure that entities do not perform randomization prior to initialization and should prevents
/// bugs like the one discussed in github.com/space-wizards/RobustToolbox/issues/3870. This test is somewhat
/// similar to <see cref="LoadSaveTicksSaveBagel"/> and <see cref="SaveLoadSave"/>, but neither of these
/// caught the mentioned bug.
/// Should ensure that entities do not perform randomization prior to initialization and should prevents
/// bugs like the one discussed in github.com/space-wizards/RobustToolbox/issues/3870. This test is somewhat
/// similar to <see cref="LoadSaveTicksSaveBagel"/> and <see cref="SaveLoadSave"/>, but neither of these
/// caught the mentioned bug.
/// </remarks>
[Test]
public async Task LoadTickLoadBagel()
@ -188,10 +200,12 @@ namespace Content.IntegrationTests.Tests
var mapSys = server.System<SharedMapSystem>();
var userData = server.ResolveDependency<IResourceManager>().UserData;
var cfg = server.ResolveDependency<IConfigurationManager>();
Assert.That(cfg.GetCVar(CCVars.GridFill), Is.False);
Assume.That(cfg.GetCVar(CCVars.GridFill), Is.False);
var testSystem = server.System<SaveLoadSaveTestSystem>();
testSystem.Enabled = true;
Assume.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the start of LoadTickLoadBagel");
MapId mapId1 = default;
MapId mapId2 = default;
var fileA = new ResPath("/load tick load a.yml");
@ -200,7 +214,7 @@ namespace Content.IntegrationTests.Tests
string yamlB;
// Load & save the first map
server.Post(() =>
await server.WaitPost(() =>
{
var path = new ResPath(TestMap);
Assert.That(mapLoader.TryLoadMap(path, out var map, out _), $"Failed to load test map {TestMap}");
@ -208,26 +222,23 @@ namespace Content.IntegrationTests.Tests
Assert.That(mapLoader.TrySaveMap(mapId1, fileA));
});
await server.WaitIdleAsync();
await using (var stream = userData.Open(fileA, FileMode.Open))
using (var reader = new StreamReader(stream))
{
yamlA = await reader.ReadToEndAsync();
}
server.RunTicks(5);
// Load & save the second map
server.Post(() =>
await server.WaitPost(() =>
{
server.RunTicks(5);
var path = new ResPath(TestMap);
Assert.That(mapLoader.TryLoadMap(path, out var map, out _), $"Failed to load test map {TestMap}");
mapId2 = map!.Value.Comp.MapId;
Assert.That(mapLoader.TrySaveMap(mapId2, fileB));
});
await server.WaitIdleAsync();
await using (var stream = userData.Open(fileB, FileMode.Open))
using (var reader = new StreamReader(stream))
{
@ -237,8 +248,12 @@ namespace Content.IntegrationTests.Tests
Assert.That(yamlA, Is.EqualTo(yamlB));
testSystem.Enabled = false;
await server.WaitPost(() => mapSys.DeleteMap(mapId1));
await server.WaitPost(() => mapSys.DeleteMap(mapId2));
await server.WaitPost(() =>
{
mapSys.DeleteMap(mapId1);
mapSys.DeleteMap(mapId2);
});
Assert.That(SEntMan.EntityCount.Equals(0), "Lingering entities at the end of LoadTickLoadBagel");
}
/// <summary>

View file

@ -18,6 +18,8 @@ public sealed class StackTest : GameTest
[Description("Tests for SharedStackSystem.SetCount.")]
public async Task SetTest()
{
Assume.That(SEntMan.EntityCount, Is.Zero, "Lingering entities at the start of the test.");
var stack = await Spawn(StackEnt1);
// Raising the count
@ -33,7 +35,7 @@ public sealed class StackTest : GameTest
Assert.That(_sStackSystem.GetCount(stack), Is.EqualTo(30));
// Setting to 0 deletes the stack
await Server.WaitPost(() => _sStackSystem.SetCount((stack, null), 0));
Server.Post(() => _sStackSystem.SetCount((stack, null), 0));
await Server.WaitRunTicks(1);
Assert.That(SEntMan.EntityCount, Is.Zero);
}
@ -42,6 +44,8 @@ public sealed class StackTest : GameTest
[Description("Tests that SharedStackSystem.MergeStacks functions as expected with small numbers.")]
public async Task MergeTest()
{
Assume.That(SEntMan.EntityCount, Is.Zero, "Lingering entities at the start of the test.");
var stacks = new HashSet<EntityUid>();
await Server.WaitPost(() =>
@ -53,10 +57,10 @@ public sealed class StackTest : GameTest
];
_sStackSystem.MergeStacks(ref stacks);
});
// Wait for the queue deletion of the empty stacks
await Server.WaitRunTicks(1);
// Need to wait for the queue deletion of the empty stacks
Server.RunTicks(1);
});
using (Assert.EnterMultipleScope())
{
@ -74,6 +78,8 @@ public sealed class StackTest : GameTest
[Description("Tests that SharedStackSystem.MergeStacks functions as expected with large numbers.")]
public async Task MergeOverflowTest()
{
Assume.That(SEntMan.EntityCount, Is.Zero, "Lingering entities at the start of the test.");
var stacks = new HashSet<EntityUid>();
await Server.WaitPost(() =>
@ -86,10 +92,10 @@ public sealed class StackTest : GameTest
];
_sStackSystem.MergeStacks(ref stacks);
});
// Wait for the queue deletion of the empty stacks
await Server.WaitRunTicks(1);
// Wait for the queue deletion of the empty stacks
Server.RunTicks(1);
});
var count = 0;
await Server.WaitPost(() =>
@ -113,20 +119,25 @@ public sealed class StackTest : GameTest
}
[Test]
[Description("Test for SharedStackSystem.TryMergeToContacts .")]
[Description("Test for SharedStackSystem.TryMergeToContacts.")]
public async Task MergeContactsTest()
{
var map = await Pair.CreateTestMap();
await Server.WaitIdleAsync();
// Spawn two stacks at the same position so they're contacting
var donor = await SpawnAtPosition(StackEnt1, map.GridCoords);
var receiver = await SpawnAtPosition(StackEnt1, map.GridCoords);
EntityUid donor = EntityUid.Invalid;
EntityUid receiver = EntityUid.Invalid;
await Server.WaitPost(() => _sStackSystem.TryMergeToContacts(donor));
await Server.WaitPost(() =>
{
donor = SSpawnAtPosition(StackEnt1, map.GridCoords);
receiver = SSpawnAtPosition(StackEnt1, map.GridCoords);
// Wait for queue deletion
await Server.WaitRunTicks(1);
_sStackSystem.TryMergeToContacts(donor);
// Wait for queue deletion
Server.RunTicks(1);
});
using (Assert.EnterMultipleScope())
{
@ -137,9 +148,11 @@ public sealed class StackTest : GameTest
}
// Now test for when there's more count than the receiver can hold
donor = await SpawnAtPosition(StackEnt30, map.GridCoords);
await Server.WaitPost(() => _sStackSystem.TryMergeToContacts(donor));
await Server.WaitPost(() =>
{
donor = SSpawnAtPosition(StackEnt30, map.GridCoords);
_sStackSystem.TryMergeToContacts(donor);
});
using (Assert.EnterMultipleScope())
{