Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ dotnet test tests/Spice86.Tests --filter 'FullyQualifiedName!~SingleStepTest'

> **Long-running test rule (Kiro agents only)**: the foreground shell tool (`execute_bash`) enforces a hard wall-clock cap (roughly 25-60s in practice) that fires *before* the `timeout` argument. When it trips it force-returns exit 1 while the detached `dotnet` process keeps running and completes normally, which looks exactly like a crash (abrupt exit 1, no test summary, no dump) but is not one. Any test run expected to exceed ~25s (the full suite is ~2 min; `CfgGraphReloadTest` alone is ~55s) MUST be launched with the background-process tool (which has no such cap), writing a completion marker, then polled with short (<20s) foreground calls until the marker appears. Do not conclude a suite "crashed" from a bare exit 1 with no summary; re-run it in the background and check the real result first. Splitting `--filter` so each foreground run stays under the cap also works.

> **Generated-code golden regeneration rule**: every `GeneratedCodeMachineTestRunner.TestGeneratedCode` call in `GeneratedCodeMachineTest` and `SpeculativeCfgTest` compares the generated C# against a committed golden under `tests/Spice86.Tests/Resources/cpuTests/res/GeneratedCode/<key>.cs.txt`, where `<key>` is the bin name plus one suffix per enabled run option (e.g. `jump1.spec`, `test386.failport.spec`, `intchain.ivt.spec`). Any change that alters generated output will fail these tests; that is intended. **Regenerate rather than hand-edit the goldens**: uncomment the `WriteExpectedGeneratedSource(...)` call in `GeneratedCodeMachineTestRunner.CompareGeneratedSourceWithExpected`, run both `GeneratedCodeMachineTest` and `SpeculativeCfgTest`, re-comment the call, then review the golden diff as the primary artefact of the change (the write goes into the source tree via `[CallerFilePath]`, one file per run). With the write enabled, run the two classes single-threaded (`-- --maximum-parallel-threads 1` or equivalent): the goldens are written and read during the run, and parallel execution can otherwise race on the same file and leave a golden unwritten. There are around fifty goldens; the `speculative_*` and ALU (`add`, `sub`, `mul`, `div`, `shifts`, ...) fixtures mostly repeat the same change, so read `jump1`, `partition_cross_function_loop`, `rep`, `div` and `selfmodifyje` closely and skim the rest as regression guards. This mirrors the existing `WriteExpectedListing`/`WriteExpectedCfgJson` workflow in `MachineTest`.

### Debugging Workflow
- **GDB Integration**: Server runs on port 10000 by default (`--GdbPort 10000`)
- Use `--Debug` to pause at startup for breakpoint setup
Expand Down
93 changes: 90 additions & 3 deletions tests/Spice86.Tests/GeneratedCodeMachineTestRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ namespace Spice86.Tests;
using Spice86.Core.Emulator.ReverseEngineer.FunctionPartitioning.Model;
using Spice86.Core.Emulator.VM;

using System.Runtime.CompilerServices;

using Xunit;

internal sealed class GeneratedCodeMachineTestRunner {
public static byte[] GetExpectedMemoryDump(string binName) {
return File.ReadAllBytes($"Resources/cpuTests/res/MemoryDumps/{binName}.bin");
Expand All @@ -27,7 +31,8 @@ public void TestGeneratedCode(string binName, byte[] expected, long maxCycles =
}

public void TestGeneratedCode(string binName, byte[] expected, GeneratedCodeRunOptions options, Action<Machine>? assertions = null) {
using CompiledGeneratedOverride compiledOverride = GenerateAndCompileSupplier(binName, options);
(CompiledGeneratedOverride compiledOverride, GeneratedCSharpProgram generatedProgram) = GenerateAndCompileSupplier(binName, options);
using CompiledGeneratedOverride ownedOverride = compiledOverride;

using Spice86Creator creator = new(binName: binName, maxCycles: options.MaxCycles, enablePit: options.EnablePit,
installInterruptVectors: options.InstallInterruptVectors, failOnUnhandledPort: options.FailOnUnhandledPort,
Expand All @@ -44,6 +49,39 @@ public void TestGeneratedCode(string binName, byte[] expected, GeneratedCodeRunO
actual.Should().Equal(expected);
}
assertions?.Invoke(spice86DependencyInjection.Machine);

// Compared last: a behavioural regression (memory dump or caller assertion) is the more
// important failure and must be the one xunit reports when both a behavioural and a golden
// mismatch occur on the same run.
CompareGeneratedSourceWithExpected(GoldenKey(binName, options), generatedProgram);
}

/// <summary>
/// Builds the golden-file key for a run: the bin file name without extension, followed by one
/// fixed suffix per enabled boolean flag in <see cref="GeneratedCodeRunOptions"/>, in a fixed
/// order. Runs of the same bin with different options therefore get distinct goldens instead of
/// overwriting each other. <c>MaxCycles</c> and <c>ConfigureMachine</c> are deliberately not part
/// of the key. Every flag is enumerated explicitly so adding one to the options is a
/// compile-visible decision here rather than a silent omission that lets two runs share a golden.
/// </summary>
public static string GoldenKey(string binName, GeneratedCodeRunOptions options) {
string key = Path.GetFileNameWithoutExtension(binName);
if (options.EnablePit) {
key += ".pit";
}
if (options.EnableA20Gate) {
key += ".a20";
}
if (options.InstallInterruptVectors) {
key += ".ivt";
}
if (options.FailOnUnhandledPort) {
key += ".failport";
}
if (options.EnableSpeculativeCfgExploration) {
key += ".spec";
}
return key;
}

public (CfgPartitionedProgram Program, GeneratedCSharpProgram GeneratedProgram) GenerateProgramAndSource(string binName, long maxCycles) {
Expand All @@ -61,6 +99,35 @@ public void TestGeneratedCode(string binName, byte[] expected, GeneratedCodeRunO
return (program, generatedProgram);
}

public void CompareGeneratedSourceWithExpected(string goldenKey, GeneratedCSharpProgram generatedProgram) {
List<string> actualLines = NormalizeLines(generatedProgram.SourceText);
//WriteExpectedGeneratedSource(goldenKey, generatedProgram.SourceText);
string resPath = Path.Join("Resources", "cpuTests", "res", "GeneratedCode", goldenKey + ".cs.txt");
File.Exists(resPath).Should().BeTrue(
$"golden '{goldenKey}.cs.txt' is missing; regenerate it by uncommenting the WriteExpectedGeneratedSource call in GeneratedCodeMachineTestRunner.CompareGeneratedSourceWithExpected, running GeneratedCodeMachineTest and SpeculativeCfgTest once, then re-commenting it");
List<string> expectedLines = NormalizeLines(File.ReadAllText(resPath));
Assert.Equal(expectedLines, actualLines);
}

private static void WriteExpectedGeneratedSource(string goldenKey, string sourceText) {
// Write directly to the source tree so the golden file is committed alongside the code.
// CallerFilePath gives the location of this file in the source tree.
string sourceDir = GetDirectoryName(GetSourceFilePath());
string resPath = Path.Join(sourceDir, "Resources", "cpuTests", "res", "GeneratedCode", goldenKey + ".cs.txt");
File.WriteAllText(resPath, sourceText);
}

private static List<string> NormalizeLines(string text) {
return text.Replace("\r\n", "\n").Split('\n').ToList();
}

private static string GetSourceFilePath([CallerFilePath] string path = "") => path;

private static string GetDirectoryName(string path) {
return Path.GetDirectoryName(path)
?? throw new InvalidOperationException($"No directory for path: {path}");
}

private static void WriteGeneratedSource(string binName, GeneratedCSharpProgram generatedProgram) {
string outputDirectory = Path.Join(AppContext.BaseDirectory, "generated-code");
Directory.CreateDirectory(outputDirectory);
Expand All @@ -71,10 +138,30 @@ private static void WriteGeneratedSource(string binName, GeneratedCSharpProgram
File.WriteAllText(Path.Join(outputDirectory, fileName + ".generated.cs"), generatedProgram.SourceText);
}

private static CompiledGeneratedOverride GenerateAndCompileSupplier(string binName, GeneratedCodeRunOptions options) {
private static (CompiledGeneratedOverride CompiledOverride, GeneratedCSharpProgram GeneratedProgram) GenerateAndCompileSupplier(string binName, GeneratedCodeRunOptions options) {
(_, GeneratedCSharpProgram generatedProgram) = new GeneratedCodeMachineTestRunner().GenerateProgramAndSource(binName, options);

return new GeneratedOverrideCompiler().CompileSupplier(generatedProgram.SourceText);
GeneratedCompilation compilation = new GeneratedOverrideCompiler().Compile(generatedProgram.SourceText);
GeneratedCodeMetrics metrics = GeneratedCodeMetricsCollector.Collect(GoldenKey(binName, options), compilation.SyntaxTree, compilation.EmitResult.Diagnostics);
WriteMetrics(metrics);
return (new GeneratedOverrideCompiler().CompileSupplier(compilation), generatedProgram);
}

private static readonly object MetricsFileLock = new();
private static bool _metricsFileInitialized;

private static void WriteMetrics(GeneratedCodeMetrics metrics) {
string outputDirectory = Path.Join(AppContext.BaseDirectory, "generated-code");
Directory.CreateDirectory(outputDirectory);
string metricsPath = Path.Join(outputDirectory, "metrics.txt");
lock (MetricsFileLock) {
if (!_metricsFileInitialized) {
File.Delete(metricsPath);
File.AppendAllText(metricsPath, GeneratedCodeMetrics.Header() + Environment.NewLine);
_metricsFileInitialized = true;
}
File.AppendAllText(metricsPath, metrics.ToLine() + Environment.NewLine);
}
}

private static CfgPartitionedProgram GenerateProgram(string binName, GeneratedCodeRunOptions options) {
Expand Down
81 changes: 81 additions & 0 deletions tests/Spice86.Tests/GeneratedCodeMetrics.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
namespace Spice86.Tests;

using System.Collections.Immutable;
using System.Linq;

/// <summary>
/// Readability metrics for a single generated-code fixture, computed from the Roslyn syntax tree and the
/// compilation diagnostics. Used by the code-generator quality harness to track how later phases change the
/// generated output. No assertions are made on these numbers yet; they run dark and are recorded to a file.
/// </summary>
internal sealed record GeneratedCodeMetrics {
public required string Fixture { get; init; }
public required int Methods { get; init; }
public required int Lines { get; init; }
public required int Labels { get; init; }
public required int GotoStatements { get; init; }
public required int DistinctGotoTargets { get; init; }
public required int CheckExternalEvents { get; init; }
public required int VerifySpeculativeEntryOrFail { get; init; }
public required int FailAsUntested { get; init; }
public required int UncheckedExpressions { get; init; }
public required int Casts { get; init; }
public required int TryStatements { get; init; }
public required int Warnings { get; init; }
public required ImmutableArray<string> WarningIds { get; init; }

// Fixed column widths so rows stay aligned regardless of the order fixtures are appended in. The
// last column (warning ids) is left unpadded because it is free text of varying length.
private const int FixtureWidth = 48;
private const int NumberWidth = 9;

private static readonly (string Header, int Width)[] Columns = [
("fixture", FixtureWidth),
("methods", NumberWidth),
("lines", NumberWidth),
("labels", NumberWidth),
("gotos", NumberWidth),
("gotoTgts", NumberWidth),
("extEvents", NumberWidth),
("specGuard", NumberWidth),
("untested", NumberWidth),
("unchecked", NumberWidth),
("casts", NumberWidth),
("tries", NumberWidth),
("warnings", NumberWidth),
("warningIds", 0)
];

public static string Header() {
return string.Concat(Columns.Select(column => Pad(column.Header, column.Width)));
}

public string ToLine() {
string warningIds = WarningIds.IsDefaultOrEmpty ? "" : string.Join(",", WarningIds);
string[] values = [
Fixture,
Methods.ToString(),
Lines.ToString(),
Labels.ToString(),
GotoStatements.ToString(),
DistinctGotoTargets.ToString(),
CheckExternalEvents.ToString(),
VerifySpeculativeEntryOrFail.ToString(),
FailAsUntested.ToString(),
UncheckedExpressions.ToString(),
Casts.ToString(),
TryStatements.ToString(),
Warnings.ToString(),
warningIds
];
return string.Concat(values.Select((value, index) => Pad(value, Columns[index].Width)));
}

private static string Pad(string value, int width) {
if (width == 0) {
return value;
}
// One trailing space guarantees a separator even when a value is wider than its column.
return value.PadRight(width - 1) + " ";
}
}
124 changes: 124 additions & 0 deletions tests/Spice86.Tests/GeneratedCodeMetricsCollector.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
namespace Spice86.Tests;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;

using System.Collections.Immutable;

/// <summary>
/// Computes <see cref="GeneratedCodeMetrics"/> from a parsed syntax tree and compilation diagnostics.
/// Counting is done on the syntax tree rather than with text regexes so it stays robust to the formatting
/// changes later phases make to the generated output.
/// </summary>
internal static class GeneratedCodeMetricsCollector {
private const string CheckExternalEventsName = "CheckExternalEvents";
private const string VerifySpeculativeEntryOrFailName = "VerifySpeculativeEntryOrFail";
private const string FailAsUntestedName = "FailAsUntested";

public static GeneratedCodeMetrics Collect(string fixture, SyntaxTree tree, ImmutableArray<Diagnostic> diagnostics) {
SyntaxNode root = tree.GetRoot();
MetricsWalker walker = new();
walker.Visit(root);

SourceText text = tree.GetText();
int lines = text.Lines.Count;

ImmutableArray<Diagnostic> warnings = diagnostics
.Where(diagnostic => diagnostic.Severity == DiagnosticSeverity.Warning)
.ToImmutableArray();
ImmutableArray<string> warningIds = warnings
.Select(diagnostic => diagnostic.Id)
.Distinct(StringComparer.Ordinal)
.OrderBy(id => id, StringComparer.Ordinal)
.ToImmutableArray();

return new GeneratedCodeMetrics {
Fixture = fixture,
Methods = walker.Methods,
Lines = lines,
Labels = walker.Labels,
GotoStatements = walker.GotoStatements,
DistinctGotoTargets = walker.GotoTargets.Count,
CheckExternalEvents = walker.CheckExternalEvents,
VerifySpeculativeEntryOrFail = walker.VerifySpeculativeEntryOrFail,
FailAsUntested = walker.FailAsUntested,
UncheckedExpressions = walker.UncheckedExpressions,
Casts = walker.Casts,
TryStatements = walker.TryStatements,
Warnings = warnings.Length,
WarningIds = warningIds
Comment thread
kevinferrare marked this conversation as resolved.
};
}

private sealed class MetricsWalker : CSharpSyntaxWalker {
public int Methods { get; private set; }
public int Labels { get; private set; }
public int GotoStatements { get; private set; }
public HashSet<string> GotoTargets { get; } = new(StringComparer.Ordinal);
public int CheckExternalEvents { get; private set; }
public int VerifySpeculativeEntryOrFail { get; private set; }
public int FailAsUntested { get; private set; }
public int UncheckedExpressions { get; private set; }
public int Casts { get; private set; }
public int TryStatements { get; private set; }

public override void VisitMethodDeclaration(MethodDeclarationSyntax node) {
Methods++;
base.VisitMethodDeclaration(node);
}

public override void VisitLabeledStatement(LabeledStatementSyntax node) {
Labels++;
base.VisitLabeledStatement(node);
}

public override void VisitGotoStatement(GotoStatementSyntax node) {
GotoStatements++;
if (node.Expression is IdentifierNameSyntax identifier) {
GotoTargets.Add(identifier.Identifier.ValueText);
}
base.VisitGotoStatement(node);
}

public override void VisitInvocationExpression(InvocationExpressionSyntax node) {
string? name = GetInvokedName(node.Expression);
if (name == CheckExternalEventsName) {
CheckExternalEvents++;
} else if (name == VerifySpeculativeEntryOrFailName) {
VerifySpeculativeEntryOrFail++;
} else if (name == FailAsUntestedName) {
FailAsUntested++;
}
base.VisitInvocationExpression(node);
}

public override void VisitCheckedExpression(CheckedExpressionSyntax node) {
if (node.Keyword.IsKind(SyntaxKind.UncheckedKeyword)) {
UncheckedExpressions++;
}
base.VisitCheckedExpression(node);
}

public override void VisitCastExpression(CastExpressionSyntax node) {
Casts++;
base.VisitCastExpression(node);
}

public override void VisitTryStatement(TryStatementSyntax node) {
TryStatements++;
base.VisitTryStatement(node);
}

private static string? GetInvokedName(ExpressionSyntax expression) {
if (expression is IdentifierNameSyntax identifier) {
return identifier.Identifier.ValueText;
}
if (expression is MemberAccessExpressionSyntax memberAccess) {
return memberAccess.Name.Identifier.ValueText;
}
return null;
}
}
}
22 changes: 22 additions & 0 deletions tests/Spice86.Tests/GeneratedCompilation.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
namespace Spice86.Tests;

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Emit;

/// <summary>
/// Result of parsing and emitting generated C# source: the parsed syntax tree, the emit result
/// (including diagnostics, which are always populated regardless of success), and the emitted PE stream.
/// </summary>
internal sealed class GeneratedCompilation {
public GeneratedCompilation(string source, SyntaxTree syntaxTree, EmitResult emitResult, MemoryStream peStream) {
Source = source;
SyntaxTree = syntaxTree;
EmitResult = emitResult;
PeStream = peStream;
}

public string Source { get; }
public SyntaxTree SyntaxTree { get; }
public EmitResult EmitResult { get; }
public MemoryStream PeStream { get; }
}
Loading
Loading