test: fix shared state, CI-vacuous tests and harness duplication (#2722)
Follow-up to #2721. It fixes how the remaining tests behave and changes no production code. ## CI - Both `dotnet test` steps pass `--filter "Category!=Manual"`. Before this, Manual-tagged tests ran on CI anyway. - The one surviving movement-throttle latency test is the only coverage of burst forgiveness over a full, wrapped history. It loses its Manual tag, so it runs on CI. ## Tests that passed on CI without testing anything - **Corpse and throwing tests.** They relied on tiledata for the weapon `Layer`, so on CI the equip silently failed and they asserted against empty lists. They now set `Layer` explicitly and assert the equip took effect. `TestThrown` also pins its own strength and range values, so a shard retuning SoulGlaive no longer breaks them. - **Healthbar poison rows.** Server.Tests registers no poisons, so every row tested `null`. They now use real poison levels. - **Account packet tests.** The mock accounts reported `Length` 0, so the character-list tests only ever encoded empty slots. They now encode the real characters. - **Pathing and AI tests that only mean something on real Trammel/Felucca terrain** are now guarded with `TileDataRequirement.SkipIfMissing()`: Britain desk and clutter scenes, house footprints (no MultiData on CI) and fingerprints. Tests that are valid on flat blank terrain stay unguarded. (A synthetic tiledata fixture to run these on CI is a planned follow-up.) - **Assertions that couldn't fail now check their claim.** Affected tests include the `DefaultWalker` cache use, cache on/off parity requiring a path, the recorder capability flags, the idle-pet gate, the monthly DST recurrence, the AdvancedSearch `Record.Exception` wrappers, `CanFitItem` with no branch-guarded assert, the Firewall callback, and `Import_NoMap`. The Geometry Parse asserts also now use the same input as their TryParse. ## Shared state Every test that changes process-wide state now restores it, even when an assert fails: - **The clock.** A new `TimerWheelScope` saves and restores `Core._tickCount`, `Core._now` and the timer wheel. `SimulatedClockScope` covers decay tests. - **StepCache settings and caches**, via `StepCacheStateScope`. - **Network state.** The ManualAllowlist, LoginAllowlist, firewall and BanChannel reporters are restored. Global packet handlers are now registered per test and removed afterwards, so they no longer stay registered for later tests. - **Data and config tables.** `ArchiveJournal` statics, `TileData.ItemTable`, `ObjectPropertyList.Enabled`, `Core.Expansion` and the localization table are restored. One `MobileStatusVersion` restore was resetting a different expansion than the one the test changed. - **Entities.** Tests with fixed serials now delete what they create. Fixed mobile serials moved to `World.NewMobile`, because deleting a hard-coded serial could unregister a live entity. - **Collection race.** `SpiralScanTests` no longer clears the spawn cache from a separate collection that could run in parallel with the cache-manager tests. - **Shared statics.** Classes reading shared static allowlists joined the sequential collection. ## Duplication New shared helpers replace copy-pasted harness code: - **Server.Tests:** `PacketAssert.Sends`, used by about 130 packet tests; `TestClientHarness`, `PersistencePipelineScope`, `NetTestHelpers`, `EntityCleanup` and one `TestMulti`. - **UOContent.Tests:** `TimerWheelScope`, which also holds one wraparound-safe `RunFor`/`RunUntil`; `AiTestStubs`, which merges six walker copies and five identical creature stubs; `MobileTestHarness`, `CommandTestHarness` and `TempDirectory`. Dead oracle packet classes are removed. ## Cost | Test | Before | After | |---|---|---| | `VirtualSerialTests` | 1.57s | 2ms (starts near the wrap via reflection instead of drawing ~17.9M serials) | | Event scheduler recurrence loop | 473ms | 1ms | | CrowdSec drain | 517ms | 16ms (event-driven instead of a fixed 500ms wait) | | AuthId tests | 248ms | 99ms (SHA2 except the two GameLogin tests) | | Container cap tests | 5,000 items | 3,449 / 3,277 (one past each protocol cap) | | Bulk removal | 100k children | 20k children | | OPL hash distribution | 60k builds | 10k builds | ## Shard-tuning decoupling - AI assertions read `ReacquireDelay` and `FollowMoveSpeed` instead of hard-coding production tuning. - The guard friendly-fire test uses an `AI_Mage` stub instead of WhiteWyrm. - Argon2 assertions pin the `argon2id` variant and `v=19` but read the cost parameters from the configured hasher. ## Tests | Run | Server.Tests | UOContent.Tests | |---|---|---| | With client files | 812 pass | 1,123 pass | | Without client files (CI simulation) | 800 pass, 12 skip | 1,021 pass, 102 skip | - The UOContent fixture reads `MODERNUO_TEST_DATA_DIR`; Server.Tests reads `MODERNUO_CLIENT_PATH`. - The 15 new UOContent skips are the map-dependent guards above. - Total run time with client files is about 5s, down from about 8s.
This commit is contained in:
Kamron Batman
GitHub
parent
79f736aa83
commit
00134cac33
135 changed files with 3164 additions and 4020 deletions
|
|
@ -13,7 +13,9 @@
|
|||
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
|
||||
*************************************************************************/
|
||||
|
||||
using System;
|
||||
using System.Net;
|
||||
using System.Reflection;
|
||||
using System.Text;
|
||||
using Server.Network;
|
||||
using Server.Network.Bans;
|
||||
|
|
@ -21,10 +23,18 @@ using Xunit;
|
|||
|
||||
namespace Server.Tests.Network.Exemptions;
|
||||
|
||||
// Addresses come from TEST-NET-1 (192.0.2.0/24) to stay clear of the other ban test classes if xUnit runs
|
||||
// them concurrently.
|
||||
public class BanExemptionsTests
|
||||
// ManualAllowlist is process-wide and BlocklistFilter reads it, so this runs in the sequential collection
|
||||
// and puts the original snapshot back. Addresses come from TEST-NET-1 (192.0.2.0/24).
|
||||
[Collection("Sequential UOContent Tests")]
|
||||
public class BanExemptionsTests : IDisposable
|
||||
{
|
||||
private static readonly FieldInfo _snapshotField =
|
||||
typeof(ManualAllowlist).GetField("_snapshot", BindingFlags.Static | BindingFlags.NonPublic);
|
||||
|
||||
private readonly object _savedSnapshot = _snapshotField.GetValue(null);
|
||||
|
||||
public void Dispose() => ManualAllowlist.LoadForTesting((BlocklistSnapshot)_savedSnapshot);
|
||||
|
||||
private static readonly IPAddress _listed = IPAddress.Parse("192.0.2.10");
|
||||
private static readonly IPAddress _unlisted = IPAddress.Parse("192.0.2.11");
|
||||
|
||||
|
|
|
|||
|
|
@ -21,14 +21,13 @@ using Xunit;
|
|||
|
||||
namespace Server.Tests.Network.Bans.Blocklist;
|
||||
|
||||
public class BlocklistFileTests
|
||||
public class BlocklistFileTests : IDisposable
|
||||
{
|
||||
private static string WriteTemp(string content)
|
||||
{
|
||||
var p = Path.Combine(Path.GetTempPath(), "bl-" + Guid.NewGuid().ToString("N") + ".txt");
|
||||
File.WriteAllText(p, content);
|
||||
return p;
|
||||
}
|
||||
private readonly TempDirectory _temp = new("bl");
|
||||
|
||||
public void Dispose() => _temp.Dispose();
|
||||
|
||||
private string WriteTemp(string content) => _temp.WriteFile("blocklist.txt", content);
|
||||
|
||||
[Fact]
|
||||
public void Reads_header_generated_and_count()
|
||||
|
|
@ -38,13 +37,12 @@ public class BlocklistFileTests
|
|||
Assert.True(h.Present);
|
||||
Assert.Equal("2026-07-21T09:24:23Z", h.Generated);
|
||||
Assert.Equal(2, h.Count);
|
||||
File.Delete(p);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Missing_file_reports_absent_and_loads_empty()
|
||||
{
|
||||
var p = Path.Combine(Path.GetTempPath(), "does-not-exist-" + Guid.NewGuid().ToString("N"));
|
||||
var p = _temp.Combine("does-not-exist.txt");
|
||||
Assert.False(BlocklistFile.TryReadHeader(p, out var h));
|
||||
Assert.False(h.Present);
|
||||
var snap = BlocklistFile.Load(p, out _, out _);
|
||||
|
|
@ -70,6 +68,5 @@ public class BlocklistFileTests
|
|||
Assert.Equal(0, skipped);
|
||||
Assert.True(snap.IsBanned(IPAddress.Parse("2.181.183.77")));
|
||||
Assert.True(snap.IsBanned(IPAddress.Parse("223.169.4.9")));
|
||||
File.Delete(p);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -20,8 +20,10 @@ using Xunit;
|
|||
|
||||
namespace Server.Tests.Network.Bans.Blocklist;
|
||||
|
||||
// No [Collection] and no static reset hook: the filter is an instance, so each test owns its own
|
||||
// snapshot and promote-guard. That is the point of it no longer being a static class.
|
||||
// The filter is an instance, so each test owns its own snapshot and promote-guard. Evaluate still
|
||||
// consults the process-wide ManualAllowlist and LoginAllowlist, which other tests rewrite, so this
|
||||
// runs in the sequential collection.
|
||||
[Collection("Sequential UOContent Tests")]
|
||||
public class BlocklistFilterTests
|
||||
{
|
||||
private static BlocklistFilter WithList(string list, bool reportHits = true, long suppressionMs = 5000)
|
||||
|
|
|
|||
|
|
@ -218,14 +218,18 @@ public class CrowdSecReporterTests
|
|||
|
||||
/// <summary>
|
||||
/// The drain task must track the loop's lifetime, not just its first await — a ValueTask-returning
|
||||
/// drain loop passed to Task.Run yields a Task<ValueTask> that completes immediately, which makes
|
||||
/// Stop()'s drain-exited handshake a no-op. With an empty queue the loop parks on WaitToReadAsync,
|
||||
/// so a correctly unwrapped task cannot win this race; a slow pool only under-detects.
|
||||
/// drain loop passed to Task.Run yields a Task<ValueTask> that completes at the first await
|
||||
/// that suspends, which makes Stop()'s drain-exited handshake a no-op. The flush delay always
|
||||
/// suspends before the first post, so once the client sees a post a wrapped task is already done.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task Start_DrainTaskSpansLoopLifetime_NotJustTheFirstAwait()
|
||||
{
|
||||
var reporter = new CrowdSecReporter(new NullAlertClient(), Settings());
|
||||
var settings = Settings();
|
||||
settings.FlushInterval = TimeSpan.FromMilliseconds(1);
|
||||
|
||||
var client = new SignalingAlertClient();
|
||||
var reporter = new CrowdSecReporter(client, settings);
|
||||
using var cts = new CancellationTokenSource();
|
||||
|
||||
reporter.Start(cts.Token);
|
||||
|
|
@ -233,14 +237,32 @@ public class CrowdSecReporterTests
|
|||
var drain = reporter.DrainTaskForTesting;
|
||||
Assert.NotNull(drain);
|
||||
|
||||
var first = await Task.WhenAny(drain, Task.Delay(TimeSpan.FromMilliseconds(500)));
|
||||
Assert.False(ReferenceEquals(first, drain), "drain task completed while the loop was still running");
|
||||
reporter.Report(IPAddress.Parse("9.9.9.9"), TimeSpan.FromHours(1), "rate-limit");
|
||||
await client.Posted.Task.WaitAsync(TimeSpan.FromSeconds(10));
|
||||
|
||||
Assert.False(drain.IsCompleted, "drain task completed while the loop was still running");
|
||||
|
||||
reporter.Stop();
|
||||
|
||||
Assert.True(drain.IsCompleted, "Stop() returned before the drain loop exited");
|
||||
}
|
||||
|
||||
private sealed class SignalingAlertClient : ICrowdSecAlertClient
|
||||
{
|
||||
public TaskCompletionSource Posted { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
|
||||
public ValueTask PostAlertsAsync(IReadOnlyList<CrowdSecAlert> alerts, CancellationToken token)
|
||||
{
|
||||
Posted.TrySetResult();
|
||||
return ValueTask.CompletedTask;
|
||||
}
|
||||
|
||||
public ValueTask DeleteDecisionsAsync(string origin, IPAddress ip, CancellationToken token) =>
|
||||
ValueTask.CompletedTask;
|
||||
|
||||
public void Dispose() { }
|
||||
}
|
||||
|
||||
private sealed class NullAlertClient : ICrowdSecAlertClient
|
||||
{
|
||||
public ValueTask PostAlertsAsync(IReadOnlyList<CrowdSecAlert> alerts, CancellationToken token) =>
|
||||
|
|
|
|||
|
|
@ -21,8 +21,11 @@ using Xunit;
|
|||
namespace Server.Tests.Network.Firewall;
|
||||
|
||||
[Collection("Sequential UOContent Tests")]
|
||||
public class FirewallPersistenceTests
|
||||
public class FirewallPersistenceTests : IDisposable
|
||||
{
|
||||
// The test host never configures the firewall, so empty is the state every other test expects.
|
||||
public void Dispose() => Server.Network.Firewall.ResetForTesting();
|
||||
|
||||
[Fact]
|
||||
public void ToSettings_RoundTrips_PermanentAndTtl()
|
||||
{
|
||||
|
|
|
|||
|
|
@ -21,8 +21,11 @@ using Xunit;
|
|||
namespace Server.Tests.Network.Firewall;
|
||||
|
||||
[Collection("Sequential UOContent Tests")]
|
||||
public class FirewallTests
|
||||
public class FirewallTests : IDisposable
|
||||
{
|
||||
// The test host never configures the firewall, so empty is the state every other test expects.
|
||||
public void Dispose() => Server.Network.Firewall.ResetForTesting();
|
||||
|
||||
private static IPAddress Ip(string s) => IPAddress.Parse(s);
|
||||
|
||||
[Fact]
|
||||
|
|
|
|||
|
|
@ -13,17 +13,74 @@
|
|||
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
|
||||
*************************************************************************/
|
||||
|
||||
using System;
|
||||
using System.Collections;
|
||||
using System.Collections.Generic;
|
||||
using System.Net;
|
||||
using System.Reflection;
|
||||
using Server.Network;
|
||||
using Server.Network.Bans;
|
||||
using Xunit;
|
||||
|
||||
namespace Server.Tests.Network.LoginAllowlists;
|
||||
|
||||
// The list is static, so every test resets it first. Addresses come from TEST-NET-3 (203.0.113.0/24) so they
|
||||
// cannot collide with the blocklist tests if xUnit runs the two classes at the same time.
|
||||
public class LoginAllowlistTests
|
||||
// The list is static and the login path and BlocklistFilter both read it, so every test resets it first,
|
||||
// the class runs in the sequential collection, and Dispose puts the original state back. Addresses come
|
||||
// from TEST-NET-3 (203.0.113.0/24).
|
||||
[Collection("Sequential UOContent Tests")]
|
||||
public class LoginAllowlistTests : IDisposable
|
||||
{
|
||||
private readonly List<(FieldInfo Field, object Value, DictionaryEntry[] Entries)> _saved = [];
|
||||
|
||||
public LoginAllowlistTests()
|
||||
{
|
||||
var fields = typeof(LoginAllowlist).GetFields(BindingFlags.Static | BindingFlags.NonPublic);
|
||||
for (var i = 0; i < fields.Length; i++)
|
||||
{
|
||||
var field = fields[i];
|
||||
if (field.IsLiteral)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
var value = field.GetValue(null);
|
||||
|
||||
// Readonly tables cannot be reassigned, so their contents are copied and refilled instead.
|
||||
DictionaryEntry[] entries = null;
|
||||
if (field.IsInitOnly && value is IDictionary table)
|
||||
{
|
||||
entries = new DictionaryEntry[table.Count];
|
||||
table.CopyTo(entries, 0);
|
||||
}
|
||||
else if (field.IsInitOnly)
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
_saved.Add((field, value, entries));
|
||||
}
|
||||
}
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
for (var i = 0; i < _saved.Count; i++)
|
||||
{
|
||||
var (field, value, entries) = _saved[i];
|
||||
if (entries == null)
|
||||
{
|
||||
field.SetValue(null, value);
|
||||
continue;
|
||||
}
|
||||
|
||||
var table = (IDictionary)value;
|
||||
table.Clear();
|
||||
for (var j = 0; j < entries.Length; j++)
|
||||
{
|
||||
table.Add(entries[j].Key, entries[j].Value);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private const long Ttl = 90 * 24 * 60 * 60; // 90 days, the shipped default
|
||||
private const long Window = 3600;
|
||||
private const long Now = 1_800_000_000;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Net;
|
||||
using Server.Accounting;
|
||||
using Server.Accounting.Security;
|
||||
|
|
@ -22,14 +23,43 @@ public class AuthIdTests : IDisposable
|
|||
IncomingAccountPackets.ClearAuthIdWindow();
|
||||
}
|
||||
|
||||
// The constructor registers each account in the process-wide Accounts table.
|
||||
private readonly List<Account> _created = [];
|
||||
|
||||
public void Dispose()
|
||||
{
|
||||
for (var i = 0; i < _created.Count; i++)
|
||||
{
|
||||
_created[i].Delete();
|
||||
}
|
||||
|
||||
IncomingAccountPackets.ClearAuthIdWindow();
|
||||
AccountSecurity.CurrentAlgorithm = _originalAlgorithm;
|
||||
}
|
||||
|
||||
private static IAccount CreateAccount(string username) =>
|
||||
Accounts.GetAccount(username) ?? new Account(username, "hunter2");
|
||||
// Auth ids never read the password, so these accounts skip the Argon2 cost.
|
||||
private IAccount CreateAccount(string username)
|
||||
{
|
||||
AccountSecurity.CurrentAlgorithm = PasswordProtectionAlgorithm.SHA2;
|
||||
try
|
||||
{
|
||||
return TrackNewAccount(username);
|
||||
}
|
||||
finally
|
||||
{
|
||||
AccountSecurity.CurrentAlgorithm = PasswordProtectionAlgorithm.Argon2;
|
||||
}
|
||||
}
|
||||
|
||||
// The game-login tests run the real password check, so they keep the configured algorithm.
|
||||
private IAccount CreateArgon2Account(string username) => TrackNewAccount(username);
|
||||
|
||||
private Account TrackNewAccount(string username)
|
||||
{
|
||||
var account = new Account(username, "hunter2");
|
||||
_created.Add(account);
|
||||
return account;
|
||||
}
|
||||
|
||||
private static int Register(IAccount account, IPAddress address) =>
|
||||
IncomingAccountPackets.RegisterAuthId(account, address, new ClientVersion(7, 0, 0, 0));
|
||||
|
|
@ -336,7 +366,7 @@ public class AuthIdTests : IDisposable
|
|||
[Fact]
|
||||
public void PreAuthenticatedGameLogin_SkipsThePasswordCheck()
|
||||
{
|
||||
var account = CreateAccount("authid-preauth-user");
|
||||
var account = CreateArgon2Account("authid-preauth-user");
|
||||
using var ns = PacketTestUtilities.CreateTestNetState();
|
||||
|
||||
// A wrong password is accepted only because the auth id already vouched for the account.
|
||||
|
|
@ -349,7 +379,7 @@ public class AuthIdTests : IDisposable
|
|||
[Fact]
|
||||
public void GameLoginWithoutPreAuthentication_StillChecksThePassword()
|
||||
{
|
||||
var account = CreateAccount("authid-nopreauth-user");
|
||||
var account = CreateArgon2Account("authid-nopreauth-user");
|
||||
using var ns = PacketTestUtilities.CreateTestNetState();
|
||||
|
||||
var wrong = new GameServer.GameLoginEventArgs(ns, account.Username, "wrong-password", false);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue