From 326dc470de487ff663eebc676049c33cefc22670 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Thu, 2 Jul 2026 22:14:03 -0700 Subject: [PATCH] fix(ci): run test projects on CI and drop impossible LowerStatReq tests CI reported success despite two failing tests because the Test step ran `dotnet test --no-restore`, but the Build step (BuildTool) only restores and builds Application - never the test projects. Without a restore, the test projects have no project.assets.json, so the Microsoft.NET.Test.Sdk build targets aren't imported, the projects aren't recognized as test projects, and `dotnet test` invokes the VSTest target against zero projects: no output, exit 0. Failures were silently masked. - Replace `--no-restore` with `dotnet test --logger trx --results-directory` on both the macOS and Linux jobs so the test projects restore and run. - Add a guard that fails the job if no .trx is produced, as a permanent backstop against silent zero-test passes. Also removes the two AosWeapon/AosArmor `EmitsLowerStatReqWhenPassed` tests. PR #2501 deliberately emits LowerStatReq (1060435) inline in each item, not in AosWeaponAttributes/AosArmorAttributes.GetProperties, which no longer take a lowerStatReq argument. A prior "fix" dropped the argument to make them compile but left the assertions expecting 1060435, so they threw KeyNotFoundException at runtime. The "not emitted by GetProperties" behavior stays covered by the sibling tests; the stale comments that claimed lowerStatReq is passed in are removed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/build-test.yml | 14 ++++++++++++-- .../AosArmorAttributesPropertiesTests.cs | 13 ------------- .../AosWeaponAttributesPropertiesTests.cs | 15 +-------------- Projects/UOContent/Misc/AOS.cs | 5 ----- 4 files changed, 13 insertions(+), 34 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 105899543..d61839a1c 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -52,7 +52,12 @@ jobs: - name: Migration Changes run: git diff --exit-code ./**/Migrations/*.v*.json - name: Test - run: dotnet test --no-restore + run: | + dotnet test --logger trx --results-directory ./TestResults + if [ -z "$(find ./TestResults -name '*.trx' 2>/dev/null)" ]; then + echo "::error::No test result files were produced - no test projects ran. Failing to avoid masking failures." + exit 1 + fi build-linux: runs-on: ubuntu-latest @@ -106,4 +111,9 @@ jobs: - name: Build run: dotnet run --project Projects/BuildTool -- --config Release --skip-prereqs - name: Test - run: dotnet test --no-restore + run: | + dotnet test --logger trx --results-directory ./TestResults + if [ -z "$(find ./TestResults -name '*.trx' 2>/dev/null)" ]; then + echo "::error::No test result files were produced - no test projects ran. Failing to avoid masking failures." + exit 1 + fi diff --git a/Projects/UOContent.Tests/Tests/PropertyList/AosArmorAttributesPropertiesTests.cs b/Projects/UOContent.Tests/Tests/PropertyList/AosArmorAttributesPropertiesTests.cs index 760bec89e..28feedbea 100644 --- a/Projects/UOContent.Tests/Tests/PropertyList/AosArmorAttributesPropertiesTests.cs +++ b/Projects/UOContent.Tests/Tests/PropertyList/AosArmorAttributesPropertiesTests.cs @@ -55,17 +55,4 @@ public class AosArmorAttributesPropertiesTests Assert.False(map.ContainsKey(1060435)); // LowerStatReq NOT read from container Assert.False(map.ContainsKey(1060410)); // DurabilityBonus excluded } - - [Fact] - public void EmitsLowerStatReqWhenPassed() - { - var attrs = new AosArmorAttributes(null) { MageArmor = 1, LowerStatReq = 50 }; - - var opl = new ObjectPropertyList(null); - attrs.GetProperties(opl); // computed value passed by the consumer, not the raw 50 - var map = Decode(opl); - - Assert.Equal("77", map[1060435]); // emitted from the param, not the container's 50 - Assert.Equal("", map[1060437]); // MageArmor still emitted - } } diff --git a/Projects/UOContent.Tests/Tests/PropertyList/AosWeaponAttributesPropertiesTests.cs b/Projects/UOContent.Tests/Tests/PropertyList/AosWeaponAttributesPropertiesTests.cs index f9f3ecdc9..ddec86fe9 100644 --- a/Projects/UOContent.Tests/Tests/PropertyList/AosWeaponAttributesPropertiesTests.cs +++ b/Projects/UOContent.Tests/Tests/PropertyList/AosWeaponAttributesPropertiesTests.cs @@ -56,19 +56,6 @@ public class AosWeaponAttributesPropertiesTests Assert.Equal("20", map[1060422]); // HitLeechHits Assert.Equal("5", map[1060438]); // MageWeapon => 30 - 25 Assert.Equal("3", map[1060450]); // SelfRepair - Assert.False(map.ContainsKey(1060435)); // LowerStatReq not emitted without the param - } - - [Fact] - public void EmitsLowerStatReqWhenPassed() - { - var attrs = new AosWeaponAttributes(null) { MageWeapon = 25 }; - - var opl = new ObjectPropertyList(null); - attrs.GetProperties(opl); // computed value passed by the weapon - var map = Decode(opl); - - Assert.Equal("40", map[1060435]); // lower requirements, in cliloc order before MageWeapon - Assert.Equal("5", map[1060438]); // MageWeapon still emitted + Assert.False(map.ContainsKey(1060435)); // LowerStatReq is emitted inline by the weapon, not by GetProperties } } diff --git a/Projects/UOContent/Misc/AOS.cs b/Projects/UOContent/Misc/AOS.cs index 9ac9158c4..6a9ec278c 100644 --- a/Projects/UOContent/Misc/AOS.cs +++ b/Projects/UOContent/Misc/AOS.cs @@ -1010,8 +1010,6 @@ namespace Server return value; } - // lowerStatReq is passed in because the weapon folds the resource's lower-requirements into - // GetLowerStatReq(); emitted in cliloc order (1060435) between the Hit* block and MageWeapon. public void GetProperties(IPropertyList list) { int prop; @@ -1200,9 +1198,6 @@ namespace Server return value; } - // lowerStatReq is passed in because consumers compute it differently: armor folds in the - // resource's ArmorLowerRequirements via GetLowerStatReq(), clothing reads it raw. Emitted in - // cliloc order (1060435) ahead of MageArmor/SelfRepair. public void GetProperties(IPropertyList list) { int prop;