From 7982b8c6500348f46ea8aa870b886f73c6f441c2 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 29 Nov 2022 14:22:47 +0100 Subject: [PATCH] Fix Full Framework tests (#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 --- build/Program.cs | 10 +++++----- .../ClrMdV1Disassembler.cs | 4 ++-- .../DataContracts.cs | 18 ++++++++++++++---- .../Disassemblers/Arm64Disassembler.cs | 4 ++-- .../Disassemblers/Arm64InstructionFormatter.cs | 4 ++-- .../Disassemblers/InstructionFormatter.cs | 4 ++-- .../Disassemblers/IntelDisassembler.cs | 4 ++-- .../Disassemblers/WindowsDisassembler.cs | 2 ++ .../BenchmarkSwitcherTest.cs | 2 +- .../DisassemblyDiagnoserTests.cs | 2 +- .../MemoryDiagnoserTests.cs | 5 +++++ .../BenchmarkDotNet.Tests/ConfigParserTests.cs | 2 +- 12 files changed, 39 insertions(+), 22 deletions(-) diff --git a/build/Program.cs b/build/Program.cs index d159745f37..82258e5d52 100644 --- a/build/Program.cs +++ b/build/Program.cs @@ -338,7 +338,7 @@ 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) @@ -346,9 +346,9 @@ public override void Run(BuildContext context) } } -[TaskName("SlowTestsNet461")] +[TaskName("SlowFullFrameworkTests")] [IsDependentOn(typeof(BuildTask))] -public class SlowTestsNet461Task : FrostingTask +public class SlowFullFrameworkTestsTask : FrostingTask { 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 { diff --git a/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs b/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs index 2eff0fe25b..29ee184df3 100644 --- a/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs +++ b/src/BenchmarkDotNet.Disassembler.x64/ClrMdV1Disassembler.cs @@ -171,11 +171,11 @@ private static IEnumerable 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 index 3585c53fd4..edcc59219d 100644 --- 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 index 7c949f576f..0aed18ae37 100644 --- a/src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs +++ b/src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs @@ -232,11 +232,11 @@ protected override IEnumerable 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 index b3f6e69325..deb557256c 100644 --- 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 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 index 0b9a3fd306..89001d0d6a 100644 --- 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 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 index 37ab4652d5..91cea04780 100644 --- a/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs +++ b/src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs @@ -86,11 +86,11 @@ protected override IEnumerable 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 index 5ed8c3b534..49e919bb63 100644 --- 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 index 0ae17a5380..7197e2302d 100644 --- 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 index 475c462764..765eac111c 100644 --- 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().All(asm => asm.IntelInstruction.ToString().Contains("ret")))); + Assert.Contains(disassemblyResult.Methods, method => method.Maps.Any(map => map.SourceCodes.OfType().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 index 7e4ea811c7..170476d30a 100755 --- 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 { { nameof(NoAllocationsAtAll.EmptyMethod), 0 } diff --git a/tests/BenchmarkDotNet.Tests/ConfigParserTests.cs b/tests/BenchmarkDotNet.Tests/ConfigParserTests.cs index e3f32cede2..a0d5c57c3a 100644 --- 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")]