Make tests automatically reset modified cvars (#28219)

* Make tests automatically reset modified cvars

* Fix bad return

* A

* Try Fix tests

* clarify comment

* update eng

diff --git a/Content.IntegrationTests/Pair/TestPair.Cvars.cs b/Content.IntegrationTests/Pair/TestPair.Cvars.cs
new file mode 100644 (file)
index 0000000..81df31f
--- /dev/null
+++ b/Content.IntegrationTests/Pair/TestPair.Cvars.cs
@@ -0,0 +1,69 @@
+#nullable enable
+using System.Collections.Generic;
+using Content.Shared.CCVar;
+using Robust.Shared.Configuration;
+using Robust.Shared.Utility;
+
+namespace Content.IntegrationTests.Pair;
+
+public sealed partial class TestPair
+{
+    private readonly Dictionary<string, object> _modifiedClientCvars = new();
+    private readonly Dictionary<string, object> _modifiedServerCvars = new();
+
+    private void OnServerCvarChanged(CVarChangeInfo args)
+    {
+        _modifiedServerCvars.TryAdd(args.Name, args.OldValue);
+    }
+
+    private void OnClientCvarChanged(CVarChangeInfo args)
+    {
+        _modifiedClientCvars.TryAdd(args.Name, args.OldValue);
+    }
+
+    internal void ClearModifiedCvars()
+    {
+        _modifiedClientCvars.Clear();
+        _modifiedServerCvars.Clear();
+    }
+
+    /// <summary>
+    /// Reverts any cvars that were modified during a test back to their original values.
+    /// </summary>
+    public async Task RevertModifiedCvars()
+    {
+        await Server.WaitPost(() =>
+        {
+            foreach (var (name, value) in _modifiedServerCvars)
+            {
+                if (Server.CfgMan.GetCVar(name).Equals(value))
+                    continue;
+                Server.Log.Info($"Resetting cvar {name} to {value}");
+                Server.CfgMan.SetCVar(name, value);
+            }
+
+            // I just love order dependent cvars
+            if (_modifiedServerCvars.TryGetValue(CCVars.PanicBunkerEnabled.Name, out var panik))
+                Server.CfgMan.SetCVar(CCVars.PanicBunkerEnabled.Name, panik);
+
+        });
+
+        await Client.WaitPost(() =>
+        {
+            foreach (var (name, value) in _modifiedClientCvars)
+            {
+                if (Client.CfgMan.GetCVar(name).Equals(value))
+                    continue;
+
+                var flags = Client.CfgMan.GetCVarFlags(name);
+                if (flags.HasFlag(CVar.REPLICATED) && flags.HasFlag(CVar.SERVER))
+                    continue;
+
+                Client.Log.Info($"Resetting cvar {name} to {value}");
+                Client.CfgMan.SetCVar(name, value);
+            }
+        });
+
+        ClearModifiedCvars();
+    }
+}

diff --git a/Content.IntegrationTests/Pair/TestPair.Recycle.cs b/Content.IntegrationTests/Pair/TestPair.Recycle.cs
index c0f4b3b745f50bffbf0f8669909ffb96082b57f0..8d1e425553b390ad61c38be35c578a8947b0291f 100644 (file)
--- a/Content.IntegrationTests/Pair/TestPair.Recycle.cs
+++ b/Content.IntegrationTests/Pair/TestPair.Recycle.cs
@@ -40,6 +40,8 @@ public sealed partial class TestPair : IAsyncDisposable
             TestMap = null;
         }
 
+        await RevertModifiedCvars();
+
         var usageTime = Watch.Elapsed;
         Watch.Restart();
         await _testOut.WriteLineAsync($"{nameof(CleanReturnAsync)}: Test borrowed pair {Id} for {usageTime.TotalMilliseconds} ms");
@@ -132,6 +134,7 @@ public sealed partial class TestPair : IAsyncDisposable
         if (gameTicker.RunLevel != GameRunLevel.PreRoundLobby)
         {
             await testOut.WriteLineAsync($"Recycling: {Watch.Elapsed.TotalMilliseconds} ms: Restarting round.");
+            Server.CfgMan.SetCVar(CCVars.GameDummyTicker, false);
             Assert.That(gameTicker.DummyTicker, Is.False);
             Server.CfgMan.SetCVar(CCVars.GameLobbyEnabled, true);
             await Server.WaitPost(() => gameTicker.RestartRound());

diff --git a/Content.IntegrationTests/Pair/TestPair.cs b/Content.IntegrationTests/Pair/TestPair.cs
index 916a94c9c4c0b7b5ad332cc6e3e05a6ebbd8d8be..7ee5dbd55c7ab2d6f819d492fc4792dcef8984dc 100644 (file)
--- a/Content.IntegrationTests/Pair/TestPair.cs
+++ b/Content.IntegrationTests/Pair/TestPair.cs
@@ -4,6 +4,7 @@ using System.IO;
 using System.Linq;
 using Content.Server.GameTicking;
 using Content.Shared.Players;
+using Robust.Shared.Configuration;
 using Robust.Shared.GameObjects;
 using Robust.Shared.IoC;
 using Robust.Shared.Network;
@@ -58,6 +59,9 @@ public sealed partial class TestPair
         (Server, ServerLogHandler) = await PoolManager.GenerateServer(settings, testOut);
         ActivateContext(testOut);
 
+        Client.CfgMan.OnCVarValueChanged += OnClientCvarChanged;
+        Server.CfgMan.OnCVarValueChanged += OnServerCvarChanged;
+
         if (!settings.NoLoadTestPrototypes)
             await LoadPrototypes(testPrototypes!);
 

diff --git a/Content.IntegrationTests/PoolManager.cs b/Content.IntegrationTests/PoolManager.cs
index 3b49ffcf8477578852aaea019583c891b8f600f5..21c93ea8aeb5359991065aaf39f4aa6f64de0a33 100644 (file)
--- a/Content.IntegrationTests/PoolManager.cs
+++ b/Content.IntegrationTests/PoolManager.cs
@@ -270,6 +270,8 @@ public static partial class PoolManager
             $"{nameof(GetServerClientPair)}: Retrieving pair {pair.Id} from pool took {poolRetrieveTime.TotalMilliseconds} ms");
         await testOut.WriteLineAsync(
             $"{nameof(GetServerClientPair)}: Returning pair {pair.Id}");
+
+        pair.ClearModifiedCvars();
         pair.Settings = poolSettings;
         pair.TestHistory.Add(currentTestName);
         pair.Watch.Restart();

diff --git a/Content.IntegrationTests/Tests/GameRules/NukeOpsTest.cs b/Content.IntegrationTests/Tests/GameRules/NukeOpsTest.cs
index 3a77834b72596a6ff27d09214b04de9be160f71c..ea19a30005ee2aafe01bd4df16e22eab629dc6cc 100644 (file)
--- a/Content.IntegrationTests/Tests/GameRules/NukeOpsTest.cs
+++ b/Content.IntegrationTests/Tests/GameRules/NukeOpsTest.cs
@@ -50,7 +50,6 @@ public sealed class NukeOpsTest
         var invSys = server.System<InventorySystem>();
         var factionSys = server.System<NpcFactionSystem>();
 
-        Assert.That(server.CfgMan.GetCVar(CCVars.GridFill), Is.False);
         server.CfgMan.SetCVar(CCVars.GridFill, true);
 
         // Initially in the lobby
@@ -200,7 +199,6 @@ public sealed class NukeOpsTest
         }
 
         ticker.SetGamePreset((GamePresetPrototype?)null);
-        server.CfgMan.SetCVar(CCVars.GridFill, false);
         await pair.SetAntagPref("NukeopsCommander", false);
         await pair.CleanReturnAsync();
     }

diff --git a/Content.Server/Administration/Systems/AdminSystem.cs b/Content.Server/Administration/Systems/AdminSystem.cs
index b7ca4e915b5d20ae66230cbb9912f7530098fecb..a1a6b33ebcb7fbb8c11eb2a6832ea8e170949963 100644 (file)
--- a/Content.Server/Administration/Systems/AdminSystem.cs
+++ b/Content.Server/Administration/Systems/AdminSystem.cs
@@ -292,6 +292,19 @@ namespace Content.Server.Administration.Systems
                 : _adminManager.ActiveAdmins;
             var hasAdmins = admins.Any();
 
+            // TODO Fix order dependent Cvars
+            // Please for the sake of my sanity don't make cvars & order dependent.
+            // Just make a bool field on the system instead of having some cvars automatically modify other cvars.
+            //
+            // I.e., this:
+            //   /sudo cvar game.panic_bunker.enabled true
+            //   /sudo cvar game.panic_bunker.disable_with_admins true
+            // and this:
+            //   /sudo cvar game.panic_bunker.disable_with_admins true
+            //   /sudo cvar game.panic_bunker.enabled true
+            //
+            // should have the same effect, but currently setting the disable_with_admins can modify enabled.
+
             if (hasAdmins && PanicBunker.DisableWithAdmins)
             {
                 _config.SetCVar(CCVars.PanicBunkerEnabled, false);