Fix Full Framework tests (dotnet#2208)

* run tests as 4.6.2, not 4.6.1 * fix tests that were broken recently and went undetected: * default TFM when using CoreRun is now net8.0 * don't load Capstone.NET types when using diassembler to avoid strong signature error * pass mandatory tfm to ClrMdV1 disassembler * disable flaky tests
arewerage · Nov 29, 2022 · 7982b8c · 7982b8c
1 parent 47b8b72
commit 7982b8c
Show file tree

Hide file tree

Showing 12 changed files with 39 additions and 22 deletions.
diff --git a/build/Program.cs b/build/Program.cs
@@ -338,17 +338,17 @@ public override bool ShouldRun(BuildContext context)
     public override void Run(BuildContext context)
     {
         var targetFrameworks = context.IsRunningOnWindows()
-            ? new[] { "net461", "net6.0" }
+            ? new[] { "net462", "net6.0" }
             : new[] { "net6.0" };
 
         foreach (var targetFramework in targetFrameworks) 
             context.RunTests(context.UnitTestsProjectFile, "UnitTests", targetFramework);
     }
 }
 
-[TaskName("SlowTestsNet461")]
+[TaskName("SlowFullFrameworkTests")]
 [IsDependentOn(typeof(BuildTask))]
-public class SlowTestsNet461Task : FrostingTask<BuildContext>
+public class SlowFullFrameworkTestsTask : FrostingTask<BuildContext>
 {
     public override bool ShouldRun(BuildContext context)
     {
@@ -357,7 +357,7 @@ public override bool ShouldRun(BuildContext context)
 
     public override void Run(BuildContext context)
     {
-        context.RunTests(context.IntegrationTestsProjectFile, "IntegrationTests", "net461");
+        context.RunTests(context.IntegrationTestsProjectFile, "IntegrationTests", "net462");
     }
 }
 
@@ -378,7 +378,7 @@ public override void Run(BuildContext context)
 
 [TaskName("AllTests")]
 [IsDependentOn(typeof(FastTestsTask))]
-[IsDependentOn(typeof(SlowTestsNet461Task))]
+[IsDependentOn(typeof(SlowFullFrameworkTestsTask))]
 [IsDependentOn(typeof(SlowTestsNet5Task))]
 public class AllTestsTask : FrostingTask<BuildContext>
 {

diff --git a/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs b/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs
@@ -171,11 +171,11 @@ private static IEnumerable<Asm> Decode(ulong startAddress, uint size, State stat
 
                 TryTranslateAddressToName(instruction, state, depth, currentMethod, out ulong referencedAddress);
 
-                yield return new Asm
+                yield return new IntelAsm
                 {
                     InstructionPointer = instruction.IP,
                     InstructionLength = instruction.Length,
-                    IntelInstruction = instruction,
+                    Instruction = instruction,
                     ReferencedAddress = (referencedAddress > ushort.MaxValue) ? referencedAddress : null,
                 };
             }

diff --git a/src/BenchmarkDotNet.Disassembler.x64/DataContracts.cs b/src/BenchmarkDotNet.Disassembler.x64/DataContracts.cs
@@ -22,15 +22,22 @@ public class Sharp : SourceCode
         public int LineNumber { get; set; }
     }
 
-    public class Asm : SourceCode
+    public abstract class Asm : SourceCode
     {
         public int InstructionLength { get; set; }
         public ulong? ReferencedAddress { get; set; }
         public bool IsReferencedAddressIndirect { get; set; }
+    }
+
+    public class IntelAsm : Asm
+    {
+        public Instruction Instruction { get; set; }
+    }
 
-        public Instruction? IntelInstruction { get; set; }
+    public class Arm64Asm : Asm
+    {
 #if !CLRMDV1
-        public Gee.External.Capstone.Arm64.Arm64Instruction Arm64Instruction { get; set; }
+        public Gee.External.Capstone.Arm64.Arm64Instruction Instruction { get; set; }
 #endif
     }
 
@@ -44,7 +51,10 @@ public class Map
         [XmlArray("Instructions")]
         [XmlArrayItem(nameof(SourceCode), typeof(SourceCode))]
         [XmlArrayItem(nameof(Sharp), typeof(Sharp))]
-        [XmlArrayItem(nameof(Asm), typeof(Asm))]
+        [XmlArrayItem(nameof(IntelAsm), typeof(IntelAsm))]
+#if NET6_0_OR_GREATER // we can replace it with !CLRMDV1 when https://github.com/9ee1/Capstone.NET/issues/36 is solved
+        [XmlArrayItem(nameof(Arm64Asm), typeof(Arm64Asm))]
+#endif
         public SourceCode[] SourceCodes { get; set; }
     }
 

diff --git a/src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs b/src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs
@@ -232,11 +232,11 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat
 
                     accumulator.Feed(instruction);
 
-                    yield return new Asm()
+                    yield return new Arm64Asm()
                     {
                         InstructionPointer = (ulong)instruction.Address,
                         InstructionLength = instruction.Bytes.Length,
-                        Arm64Instruction = instruction,
+                        Instruction = instruction,
                         ReferencedAddress = (address > ushort.MaxValue) ? address : null,
                         IsReferencedAddressIndirect = isIndirect
                     };

diff --git a/src/BenchmarkDotNet/Disassemblers/Arm64InstructionFormatter.cs b/src/BenchmarkDotNet/Disassemblers/Arm64InstructionFormatter.cs
@@ -9,11 +9,11 @@ internal static class Arm64InstructionFormatter
     {
         // FormatterOptions is an Intel-specific concept that comes from the Iced library, but since our users can pass custom
         // Iced Formatter to DisassemblyDiagnoserConfig and it provides all the settings we need, we just reuse it here.
-        internal static string Format(Asm asm, FormatterOptions formatterOptions,
+        internal static string Format(Arm64Asm asm, FormatterOptions formatterOptions,
             bool printInstructionAddresses, uint pointerSize, IReadOnlyDictionary<ulong, string> symbols)
         {
             StringBuilder output = new ();
-            Arm64Instruction instruction = asm.Arm64Instruction;
+            Arm64Instruction instruction = asm.Instruction;
 
             if (printInstructionAddresses)
             {

diff --git a/src/BenchmarkDotNet/Disassemblers/InstructionFormatter.cs b/src/BenchmarkDotNet/Disassemblers/InstructionFormatter.cs
@@ -9,8 +9,8 @@ internal static class CodeFormatter
         internal static string Format(SourceCode sourceCode, Formatter formatter, bool printInstructionAddresses, uint pointerSize, IReadOnlyDictionary<ulong, string> symbols)
             => sourceCode switch
             {
-                Asm asm when asm.IntelInstruction.HasValue => IntelInstructionFormatter.Format(asm.IntelInstruction.Value, formatter, printInstructionAddresses, pointerSize),
-                Asm asm when asm.Arm64Instruction is not null => Arm64InstructionFormatter.Format(asm, formatter.Options, printInstructionAddresses, pointerSize, symbols),
+                IntelAsm intel => IntelInstructionFormatter.Format(intel.Instruction, formatter, printInstructionAddresses, pointerSize),
+                Arm64Asm arm64 => Arm64InstructionFormatter.Format(arm64, formatter.Options, printInstructionAddresses, pointerSize, symbols),
                 Sharp sharp => sharp.Text,
                 MonoCode mono => mono.Text,
                 _ => throw new NotSupportedException(),

diff --git a/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs b/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs
@@ -86,11 +86,11 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat
                     }
                 }
 
-                yield return new Asm
+                yield return new IntelAsm
                 {
                     InstructionPointer = instruction.IP,
                     InstructionLength = instruction.Length,
-                    IntelInstruction = instruction,
+                    Instruction = instruction,
                     ReferencedAddress = (address > ushort.MaxValue) ? address : null,
                     IsReferencedAddressIndirect = isIndirect,
                 };

diff --git a/src/BenchmarkDotNet/Disassemblers/WindowsDisassembler.cs b/src/BenchmarkDotNet/Disassemblers/WindowsDisassembler.cs
@@ -146,6 +146,8 @@ private string BuildArguments(DiagnoserActionParameters parameters, string resul
                 .Append(' ')
                 .Append(config.Syntax.ToString())
                 .Append(' ')
+                .Append(parameters.BenchmarkCase.Job.Environment.GetRuntime().MsBuildMoniker)
+                .Append(' ')
                 .Append(string.Join(" ", config.Filters.Select(Escape)))
                 .ToString();
 

diff --git a/tests/BenchmarkDotNet.IntegrationTests/BenchmarkSwitcherTest.cs b/tests/BenchmarkDotNet.IntegrationTests/BenchmarkSwitcherTest.cs
@@ -332,7 +332,7 @@ public void WhenUserCreatesStaticBenchmarkMethodWeDisplayAnError_FromAssembly()
             Assert.Contains("static", logger.GetLog());
         }
 
-        [Fact]
+        [FactDotNetCoreOnly("For some reason this test is flaky on Full Framework")]
         public void WhenUserAddTheResumeAttributeAndRunTheBenchmarks()
         {
             var logger = new OutputLogger(Output);

diff --git a/tests/BenchmarkDotNet.IntegrationTests/DisassemblyDiagnoserTests.cs b/tests/BenchmarkDotNet.IntegrationTests/DisassemblyDiagnoserTests.cs
@@ -143,7 +143,7 @@ public void CanDisassembleInlinableBenchmarks(Jit jit, Platform platform, Runtim
 
             var disassemblyResult = disassemblyDiagnoser.Results.Values.Single(result => result.Methods.Count(method => method.Name.Contains(nameof(WithInlineable.JustReturn))) == 1);
 
-            Assert.Contains(disassemblyResult.Methods, method => method.Maps.Any(map => map.SourceCodes.OfType<Asm>().All(asm => asm.IntelInstruction.ToString().Contains("ret"))));
+            Assert.Contains(disassemblyResult.Methods, method => method.Maps.Any(map => map.SourceCodes.OfType<IntelAsm>().All(asm => asm.Instruction.ToString().Contains("ret"))));
         }
 
         private IConfig CreateConfig(Jit jit, Platform platform, Runtime runtime, IDiagnoser disassemblyDiagnoser, RunStrategy runStrategy)

diff --git a/tests/BenchmarkDotNet.IntegrationTests/MemoryDiagnoserTests.cs b/tests/BenchmarkDotNet.IntegrationTests/MemoryDiagnoserTests.cs
@@ -123,6 +123,11 @@ [Benchmark] public void EmptyMethod() { }
         [Trait(Constants.Category, Constants.BackwardCompatibilityCategory)]
         public void EngineShouldNotInterfereAllocationResults(IToolchain toolchain)
         {
+            if (RuntimeInformation.IsFullFramework && toolchain.IsInProcess)
+            {
+                return; // this test is flaky on Full Framework
+            }
+
             AssertAllocations(toolchain, typeof(NoAllocationsAtAll), new Dictionary<string, long>
             {
                 { nameof(NoAllocationsAtAll.EmptyMethod), 0 }

diff --git a/tests/BenchmarkDotNet.Tests/ConfigParserTests.cs b/tests/BenchmarkDotNet.Tests/ConfigParserTests.cs
@@ -160,7 +160,7 @@ public void SpecifyingCoreRunWithFullFrameworkTargetsMostRecentTfm()
 
             CoreRunToolchain coreRunToolchain = (CoreRunToolchain)coreRunJob.GetToolchain();
             DotNetCliGenerator generator = (DotNetCliGenerator)coreRunToolchain.Generator;
-            Assert.Equal("net7.0", generator.TargetFrameworkMoniker);
+            Assert.Equal("net8.0", generator.TargetFrameworkMoniker);
         }
 
         [FactDotNetCoreOnly("It's impossible to determine TFM for CoreRunToolchain if host process is not .NET (Core) process")]