From 420f2460316d05d9a021671d24cbb803a9ea3a8c Mon Sep 17 00:00:00 2001 From: sakanni Date: Thu, 20 Aug 2026 01:13:55 +0100 Subject: [PATCH] fix: order assembly loading deterministically, addressing CI_Toolkit#161's mechanism --- .../AssemblyLoadOrderTests.cs | 82 +++++++++++++++++++ .../VersioningRunner/Commands/RunCommand.cs | 21 ++++- 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs diff --git a/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs b/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs new file mode 100644 index 0000000..30f6d8d --- /dev/null +++ b/tools/VersioningRunner/src/VersioningRunner.Tests/AssemblyLoadOrderTests.cs @@ -0,0 +1,82 @@ +using VersioningRunner.Commands; +using Xunit; + +namespace VersioningRunner.Tests +{ + // Load order is not cosmetic. ProbeDeclaringType takes its verdict from the first + // loaded assembly that yields the declaring type, and 42 type names in the fleet are + // defined by more than one assembly, so the enumeration order decides the + // classification for those. See CI_Toolkit#161. + public class AssemblyLoadOrderTests + { + private static string[] Names(IEnumerable paths) + => paths.Select(Path.GetFileName).ToArray()!; + + [Fact] + public void UnsortedInput_IsOrdered() + { + string[] input = + [ + @"C:\p\Structure_Engine.dll", + @"C:\p\Acoustic_oM.dll", + @"C:\p\Revit_Core_Engine_2022.dll", + @"C:\p\BHoM.dll", + ]; + + Assert.Equal( + ["Acoustic_oM.dll", "BHoM.dll", "Revit_Core_Engine_2022.dll", "Structure_Engine.dll"], + Names(RunCommand.OrderForLoad(input))); + } + + // The discriminating case, and the reason the comparer is named rather than + // implied. OrdinalIgnoreCase uppercases before comparing, so '_' (0x5F) lands + // after letters and RevitAPIUI precedes Revit_Adapter. A lowercase-based sort + // ('_' before 'a') reverses the pair. Both are "case-insensitive"; only one + // matches what the runner observed on NTFS, and picking the wrong one would + // change which assembly answers for a contested type. + [Theory] + [InlineData("RevitAPIUI.dll", "Revit_Adapter.dll")] + [InlineData("TestRunner.dll", "Test_Engine.dll")] + [InlineData("UIFrameworkServices.dll", "UI_Engine.dll")] + public void UnderscoreSortsAfterLetters_MatchingObservedNtfsOrder(string first, string second) + { + // Supplied in the opposite order to the expected result, so a missing sort fails. + string[] input = [$@"C:\p\{second}", $@"C:\p\{first}"]; + Assert.Equal([first, second], Names(RunCommand.OrderForLoad(input))); + } + + // Excludes plain Ordinal, which the underscore cases above do not. Measured on the + // real 132-assembly closure: Ordinal, OrdinalIgnoreCase and a lowercase-based sort + // all produce different orders, and only OrdinalIgnoreCase reproduces what NTFS + // returned. This is the pair where Ordinal diverges first. + [Fact] + public void ComparerIsOrdinalIgnoreCase_NotOrdinal() + { + string[] input = [@"C:\p\Accord.MachineLearning.dll", @"C:\p\Accord.dll"]; + Assert.Equal(["Accord.dll", "Accord.MachineLearning.dll"], Names(RunCommand.OrderForLoad(input))); + } + + [Fact] + public void OrderIsIndependentOfInputOrder() + { + string[] a = [@"C:\p\B_oM.dll", @"C:\p\A_Engine.dll", @"C:\p\C_Adapter.dll"]; + string[] b = [@"C:\p\C_Adapter.dll", @"C:\p\B_oM.dll", @"C:\p\A_Engine.dll"]; + + Assert.Equal(Names(RunCommand.OrderForLoad(a)), Names(RunCommand.OrderForLoad(b))); + } + + [Fact] + public void SortsOnFileNameNotFullPath() + { + // Directory.GetFiles is not recursive so this cannot arise today, but sorting + // whole paths would order by directory first and silently change the answer + // if it ever did. + string[] input = [@"C:\zzz\A_Engine.dll", @"C:\aaa\B_Engine.dll"]; + Assert.Equal(["A_Engine.dll", "B_Engine.dll"], Names(RunCommand.OrderForLoad(input))); + } + + [Fact] + public void EmptyInput_IsEmpty() + => Assert.Empty(RunCommand.OrderForLoad([])); + } +} diff --git a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs index bf38485..dc333ed 100644 --- a/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs +++ b/tools/VersioningRunner/src/VersioningRunner/Commands/RunCommand.cs @@ -281,10 +281,29 @@ internal static int ExitCodeFor(VersioningStatus status) private static readonly Regex _revitAssemblyPattern = new(@"Revit_\w+20\d{2}\.dll$", RegexOptions.IgnoreCase | RegexOptions.Compiled); + // Load order decides classification, so it must not be left to the filesystem. + // + // ProbeDeclaringType walks the loaded list and takes its verdict from the FIRST + // assembly that yields the declaring type. Where two repos declare the same type, + // whichever is enumerated first decides whether the finding reads as a genuine + // regression or as an infrastructure problem. That is CI_Toolkit#161's mechanism: + // BH.Revit.Engine.Core.Compute is defined by both Revit_Core_Engine and + // Revit_ModelQA_Engine, and 42 such type-level collisions exist across the fleet. + // + // Directory.GetFiles documents no ordering. Measured on windows-2025-vs2026 across + // four cold-rebuild runs on separate runners, NTFS returned exactly + // StringComparer.OrdinalIgnoreCase order every time (132 and 111 entries), so + // sorting is a no-op there and this is defensive rather than corrective. The + // comparer is not interchangeable: OrdinalIgnoreCase uppercases before comparing, + // which puts '_' (0x5F) after letters, so RevitAPIUI sorts before Revit_Adapter. + // A lowercase-based sort reverses that pair and would change which assembly answers. + internal static IReadOnlyList OrderForLoad(IEnumerable files) + => files.OrderBy(f => Path.GetFileName(f), StringComparer.OrdinalIgnoreCase).ToArray(); + private static List LoadAssemblies(string folder) { var loaded = new List(); - foreach (string file in Directory.GetFiles(folder)) + foreach (string file in OrderForLoad(Directory.GetFiles(folder))) { if (IsLoadableAssembly(file)) {