From 6179e5b1465fb7812807566daad6989d80fd50e1 Mon Sep 17 00:00:00 2001 From: Chris Rudolphi <1702962+clrudolphi@users.noreply.github.com> Date: Wed, 24 Jun 2026 08:54:37 -0500 Subject: [PATCH 1/3] Fix #1083 NullReferenceException in CucumberMessageFactory.ToTestStepStarted during scenario cleanup causes scenario to be silently omitted from HTML report. This is caused when scenario execution stops part-way through the scenario's steps, causing the Formatters' capture of the test case structure to fail. A retry of the scenario would then fail as the code assumed that the first attempt had captured all steps and hooks of the scenario. --- .gitignore | 1 + CHANGELOG.md | 3 +- .../HookStepExecutionTracker.cs | 11 ++- .../ExecutionTracking/HookStepTracker.cs | 2 +- .../ITestCaseExecutionTrackerFactory.cs | 1 - .../PickleExecutionTracker.cs | 29 ++++++- .../Formatters/ExecutionTracking/StepKind.cs | 10 +++ .../ExecutionTracking/StepTrackerBase.cs | 12 ++- .../TestCaseExecutionTracker.cs | 33 +++++--- .../TestCaseExecutionTrackerFactory.cs | 2 - .../ExecutionTracking/TestCaseTracker.cs | 49 +++++++----- .../TestStepExecutionTracker.cs | 19 ++--- .../ExecutionTracking/TestStepTracker.cs | 19 +++-- .../HookStepExecutionTrackerTests.cs | 5 +- .../PickleExecutionTrackerTests.cs | 52 ++++++++++++ .../ExecutionTracking/TestCaseTrackerTests.cs | 80 +++++++++++++++++++ .../TestStepExecutionTrackerTests.cs | 7 +- .../ExecutionTracking/TestStepTrackerTests.cs | 14 ++-- 18 files changed, 275 insertions(+), 74 deletions(-) create mode 100644 Reqnroll/Formatters/ExecutionTracking/StepKind.cs create mode 100644 Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestCaseTrackerTests.cs diff --git a/.gitignore b/.gitignore index aebd17056..c6369be8c 100644 --- a/.gitignore +++ b/.gitignore @@ -381,3 +381,4 @@ nCrunchTemp*.csproj *.received.* +/.claude diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e9eef7bf..48b9462c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,9 @@ * Updated Cucumber dependencies to: Gherkin v39.1.0, Cucumber.Messages v32.0.1 and Cucumber.HtmlFormatter v23.1.0. Formatters.Tests modified by adopting use of Cucumber/CCK (v29.2.2). (#984) ## Bug fixes: +* Fix: A scenario wtih a Retry mechanism (such as NUnitRetry.ReqnrollPlugin) and with 'StopAtFirstError' enabled, can cause a null reference exception in the Cucumber Formatters. (#1083) -*Contributors of this release (in alphabetical order):* +*Contributors of this release (in alphabetical order):* @clrudolphi # v3.3.4 - 2026-03-23 diff --git a/Reqnroll/Formatters/ExecutionTracking/HookStepExecutionTracker.cs b/Reqnroll/Formatters/ExecutionTracking/HookStepExecutionTracker.cs index 827d4fa47..8c989f42f 100644 --- a/Reqnroll/Formatters/ExecutionTracking/HookStepExecutionTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/HookStepExecutionTracker.cs @@ -18,12 +18,11 @@ public async Task ProcessEvent(HookBindingStartedEvent hookBindingStartedEvent) var hookId = PickleExecutionTracker.StepDefinitionsByBinding[hookBindingStartedEvent.HookBinding]; - if (ParentTracker.IsFirstAttempt) - { - TestCaseTracker.ProcessEvent(hookBindingStartedEvent); - } - - StepTracker = PickleExecutionTracker.TestCaseTracker.GetHookStepTrackerByHookId(hookId); + // Resolve (or create on first sight) the ledger entry for this hook firing. The occurrence index + // distinguishes repeated firings of the same hook (e.g. BeforeStep/AfterStep) and stays stable + // across retries, even if an earlier attempt aborted before this firing occurred. + var occurrence = ParentTracker.NextOccurrence(StepKind.Hook, hookId); + StepTracker = TestCaseTracker.GetOrCreateHookStepTracker(hookId, occurrence); await Publisher.PublishAsync(Envelope.Create(MessageFactory.ToTestStepStarted(this))); } diff --git a/Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs b/Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs index eb46a1672..504a16d83 100644 --- a/Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs @@ -4,7 +4,7 @@ /// Tracks the information needed for a Cucumber Messages "hook step", that is a hook with binding information. /// The hook step needs to be built upon the first execution attempt of a pickle. /// -public class HookStepTracker(string testStepId, string hookId) : StepTrackerBase(testStepId) +public class HookStepTracker(string testStepId, string hookId, int occurrence) : StepTrackerBase(testStepId, occurrence) { public string HookId { get; } = hookId; } \ No newline at end of file diff --git a/Reqnroll/Formatters/ExecutionTracking/ITestCaseExecutionTrackerFactory.cs b/Reqnroll/Formatters/ExecutionTracking/ITestCaseExecutionTrackerFactory.cs index ee6d61487..fa649f5fb 100644 --- a/Reqnroll/Formatters/ExecutionTracking/ITestCaseExecutionTrackerFactory.cs +++ b/Reqnroll/Formatters/ExecutionTracking/ITestCaseExecutionTrackerFactory.cs @@ -8,6 +8,5 @@ TestCaseExecutionTracker CreateTestCaseExecutionTracker( IPickleExecutionTracker parentTracker, int attemptId, string testCaseId, - TestCaseTracker testCaseTracker, IMessagePublisher picklePublisher); } diff --git a/Reqnroll/Formatters/ExecutionTracking/PickleExecutionTracker.cs b/Reqnroll/Formatters/ExecutionTracking/PickleExecutionTracker.cs index ef3fd6cb2..eba6b3b94 100644 --- a/Reqnroll/Formatters/ExecutionTracking/PickleExecutionTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/PickleExecutionTracker.cs @@ -114,6 +114,9 @@ private async Task FlushBuffer() public int AttemptCount { get; private set; } public bool Finished { get; private set; } + // Guards once-only publication of the TestCase (definition) message for this pickle. + private bool _testCaseMessagePublished; + private bool HasCurrentTestCaseExecution => Enabled && CurrentTestCaseExecutionTracker != null; public ScenarioExecutionStatus ScenarioExecutionStatus => _executionHistory.Last().ScenarioExecutionStatus; @@ -166,9 +169,25 @@ public async Task FinalizeTracking() if (!HasCurrentTestCaseExecution) return; + // If every attempt failed (so we never hit a passing/skipped terminal state above), publish the + // TestCase now from the accumulated ledger, which holds the union of all attempts. + await PublishTestCaseMessageOnce(); + await CurrentTestCaseExecutionTracker.FinalizeTracking(); } + // Publishes the TestCase (definition) message exactly once per pickle. The per-pickle + // OrderFixingMessagePublisher guarantees this message is ordered ahead of the TestCaseStarted / + // TestStep* messages that reference its ids, regardless of when this is called. + private async Task PublishTestCaseMessageOnce() + { + if (_testCaseMessagePublished || !Enabled) + return; + + _testCaseMessagePublished = true; + await _publisher.PublishAsync(Envelope.Create(_messageFactory.ToTestCase(TestCaseTracker))); + } + public async Task ProcessEvent(ScenarioStartedEvent scenarioStartedEvent) { if (!Enabled) @@ -189,7 +208,7 @@ public async Task ProcessEvent(ScenarioStartedEvent scenarioStartedEvent) CurrentTestCaseExecutionTracker = null; // will be set again in SetExecutionRecordAsCurrentlyExecuting a few lines below } - var testCaseExecutionTracker = _testCaseExecutionTrackerFactory.CreateTestCaseExecutionTracker(this, AttemptCount, TestCaseId, TestCaseTracker, _publisher); + var testCaseExecutionTracker = _testCaseExecutionTrackerFactory.CreateTestCaseExecutionTracker(this, AttemptCount, TestCaseId, _publisher); SetExecutionRecordAsCurrentlyExecuting(testCaseExecutionTracker); await testCaseExecutionTracker.ProcessEvent(scenarioStartedEvent); } @@ -201,6 +220,14 @@ public async Task ProcessEvent(ScenarioFinishedEvent scenarioFinishedEvent) Finished = true; await CurrentTestCaseExecutionTracker.ProcessEvent(scenarioFinishedEvent); + + // A passing or cleanly-skipped attempt executed every step and hook, so the ledger is complete and + // correctly ordered, and no retry will follow: it is safe to publish the TestCase now. Anything that + // might still be retried (TestError, pending, undefined, ambiguous) is deferred to FinalizeTracking, + // by which point the ledger holds the union of all attempts. + var status = scenarioFinishedEvent.ScenarioContext.ScenarioExecutionStatus; + if (status is ScenarioExecutionStatus.OK or ScenarioExecutionStatus.Skipped) + await PublishTestCaseMessageOnce(); } public async Task ProcessEvent(StepStartedEvent stepStartedEvent) diff --git a/Reqnroll/Formatters/ExecutionTracking/StepKind.cs b/Reqnroll/Formatters/ExecutionTracking/StepKind.cs new file mode 100644 index 000000000..78656b44c --- /dev/null +++ b/Reqnroll/Formatters/ExecutionTracking/StepKind.cs @@ -0,0 +1,10 @@ +namespace Reqnroll.Formatters.ExecutionTracking; + +/// +/// Discriminates the two kinds of tracked test-case steps for identity/occurrence keying. +/// +public enum StepKind +{ + TestStep, + Hook +} diff --git a/Reqnroll/Formatters/ExecutionTracking/StepTrackerBase.cs b/Reqnroll/Formatters/ExecutionTracking/StepTrackerBase.cs index 7ee75fc65..e861bb9f1 100644 --- a/Reqnroll/Formatters/ExecutionTracking/StepTrackerBase.cs +++ b/Reqnroll/Formatters/ExecutionTracking/StepTrackerBase.cs @@ -1,6 +1,14 @@ -namespace Reqnroll.Formatters.ExecutionTracking; +namespace Reqnroll.Formatters.ExecutionTracking; -public abstract class StepTrackerBase(string testStepId) +public abstract class StepTrackerBase(string testStepId, int occurrence) { public string TestStepId { get; } = testStepId; + + /// + /// The 1-based index of this step/hook among occurrences of the same identity + /// (PickleStepId for steps, HookId for hooks) within a single execution attempt. + /// This keeps the ledger entry identity stable across retries even when an earlier + /// attempt aborted before this occurrence ran. + /// + public int Occurrence { get; } = occurrence; } diff --git a/Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTracker.cs b/Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTracker.cs index 582f359df..7e54c333e 100644 --- a/Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTracker.cs @@ -15,9 +15,13 @@ namespace Reqnroll.Formatters.ExecutionTracking; public class TestCaseExecutionTracker { private readonly ICucumberMessageFactory _messageFactory; - private readonly TestCaseTracker _testCaseTracker; private readonly Stack _stepExecutionTrackers; + // Per-attempt occurrence counters keyed on step/hook identity. Because a fresh + // TestCaseExecutionTracker is created for each attempt, these counters are naturally + // scoped to a single attempt and reset on every retry. + private readonly Dictionary<(StepKind Kind, string Id), int> _occurrenceCounters = new(); + public IPickleExecutionTracker ParentTracker { get; } public int AttemptId { get; } @@ -40,11 +44,10 @@ public class TestCaseExecutionTracker public bool IsFirstAttempt => AttemptId == 0; - public TestCaseExecutionTracker(IPickleExecutionTracker parentTracker, int attemptId, string testCaseStartedId, string testCaseId, TestCaseTracker testCaseTracker, ICucumberMessageFactory messageFactory, IMessagePublisher publisher, IStepTrackerFactory stepTrackerFactory) + public TestCaseExecutionTracker(IPickleExecutionTracker parentTracker, int attemptId, string testCaseStartedId, string testCaseId, ICucumberMessageFactory messageFactory, IMessagePublisher publisher, IStepTrackerFactory stepTrackerFactory) { _messageFactory = messageFactory; _stepExecutionTrackers = new(); - _testCaseTracker = testCaseTracker; ParentTracker = parentTracker; AttemptId = attemptId; TestCaseStartedId = testCaseStartedId; @@ -53,6 +56,18 @@ public TestCaseExecutionTracker(IPickleExecutionTracker parentTracker, int attem _stepTrackerFactory = stepTrackerFactory; } + /// + /// Returns the next 1-based occurrence index for the given step/hook identity within this attempt. + /// Called exactly once per StepStarted / HookBindingStarted event. + /// + internal int NextOccurrence(StepKind kind, string id) + { + var key = (kind, id); + var next = _occurrenceCounters.TryGetValue(key, out var current) ? current + 1 : 1; + _occurrenceCounters[key] = next; + return next; + } + public async Task ProcessEvent(ScenarioStartedEvent scenarioStartedEvent) { TestCaseStartedTimestamp = scenarioStartedEvent.Timestamp; @@ -62,14 +77,10 @@ public async Task ProcessEvent(ScenarioStartedEvent scenarioStartedEvent) public async Task ProcessEvent(ScenarioFinishedEvent scenarioFinishedEvent) { - // If this is the first attempt, at ScenarioFinished, we have enough information about which Hook and Step Bindings were used; - // we can now generate the TestCase message and publish it. - if (AttemptId == 0) - { - var testCase = _messageFactory.ToTestCase(_testCaseTracker); - await _publisher.PublishAsync(Envelope.Create(testCase)); // using the OrderFixingMessagePublisher will ensure that this is published before any other messages related to this TestCase (such as TestCaseStarted, etc) - } - + // Note: publication of the TestCase (definition) message is owned by the PickleExecutionTracker, + // which emits it once the ledger is known to be complete (a passing/skipped attempt, or at finalize). + // It cannot be published here at attempt 0, because an aborted attempt 0 may not yet have reached + // every step/hook; later attempts can still append newly-discovered entries to the ledger. TestCaseFinishedTimestamp = scenarioFinishedEvent.Timestamp; ScenarioExecutionStatus = scenarioFinishedEvent.ScenarioContext.ScenarioExecutionStatus; diff --git a/Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTrackerFactory.cs b/Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTrackerFactory.cs index 7af7f7474..97235a0c0 100644 --- a/Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTrackerFactory.cs +++ b/Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTrackerFactory.cs @@ -10,7 +10,6 @@ public TestCaseExecutionTracker CreateTestCaseExecutionTracker( IPickleExecutionTracker parentTracker, int attemptId, string testCaseId, - TestCaseTracker testCaseTracker, IMessagePublisher picklePublisher = null) { return new TestCaseExecutionTracker( @@ -18,7 +17,6 @@ public TestCaseExecutionTracker CreateTestCaseExecutionTracker( attemptId, idGenerator.GetNewId(), testCaseId, - testCaseTracker, messageFactory, picklePublisher ?? publisher, stepTrackerFactory); diff --git a/Reqnroll/Formatters/ExecutionTracking/TestCaseTracker.cs b/Reqnroll/Formatters/ExecutionTracking/TestCaseTracker.cs index 181eddaf4..0e939357e 100644 --- a/Reqnroll/Formatters/ExecutionTracking/TestCaseTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/TestCaseTracker.cs @@ -1,5 +1,4 @@ -using Reqnroll.Events; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; using Reqnroll.Bindings; @@ -8,7 +7,8 @@ namespace Reqnroll.Formatters.ExecutionTracking; /// /// Tracks the information needed for a Cucumber Messages "test case", that is a pickle with binding information, /// so it captures for every step and hook the related step definitions. -/// The test case needs to be built upon the first execution attempt of a pickle. +/// The ledger is populated lazily as steps/hooks are first seen across execution attempts; an entry for a +/// step/hook reached only on a later (retry) attempt is appended when it is first encountered. /// public class TestCaseTracker(string testCaseId, string pickleId, IPickleExecutionTracker parentTracker) { @@ -23,31 +23,38 @@ internal string FindStepDefinitionIdByBindingKey(IBinding binding) return ParentTracker.StepDefinitionsByBinding[binding]; } - public TestStepTracker GetTestStepTrackerByPickleId(string pickleId) + /// + /// Returns the ledger entry for the -th execution of pickle step + /// within an attempt, creating and appending it on first sight. + /// This is stable across retries: a step first reached on a later (less-truncated) attempt is + /// simply appended, and a step already seen on an earlier attempt reuses its existing entry (and id). + /// + public TestStepTracker GetOrCreateTestStepTracker(string pickleStepId, int occurrence) { - return Steps.OfType().FirstOrDefault(sd => sd.PickleStepId == pickleId); - } + var existing = Steps.OfType() + .FirstOrDefault(sd => sd.PickleStepId == pickleStepId && sd.Occurrence == occurrence); + if (existing != null) + return existing; - public HookStepTracker GetHookStepTrackerByHookId(string hookId) - { - return Steps.OfType().First(sd => sd.HookId == hookId); - } - - public void ProcessEvent(StepStartedEvent stepStartedEvent) - { - var pickleStepId = stepStartedEvent.StepContext.StepInfo.PickleStepId; - var testStepId = ParentTracker.IdGenerator.GetNewId(); - var stepTracker = new TestStepTracker(testStepId, pickleStepId, this); + var stepTracker = new TestStepTracker(ParentTracker.IdGenerator.GetNewId(), pickleStepId, occurrence, this); Steps.Add(stepTracker); - stepTracker.ProcessEvent(stepStartedEvent); + return stepTracker; } - public void ProcessEvent(HookBindingStartedEvent hookBindingStartedEvent) + /// + /// Hook counterpart of , keyed on (, ). + /// The occurrence index distinguishes repeated firings of the same hook binding (e.g. a BeforeStep/AfterStep + /// hook that runs once per step) so that each firing maps to its own test step. + /// + public HookStepTracker GetOrCreateHookStepTracker(string hookId, int occurrence) { - var hookId = ParentTracker.StepDefinitionsByBinding[hookBindingStartedEvent.HookBinding]; + var existing = Steps.OfType() + .FirstOrDefault(sd => sd.HookId == hookId && sd.Occurrence == occurrence); + if (existing != null) + return existing; - var testStepId = ParentTracker.IdGenerator.GetNewId(); - var hookStepTracker = new HookStepTracker(testStepId, hookId); + var hookStepTracker = new HookStepTracker(ParentTracker.IdGenerator.GetNewId(), hookId, occurrence); Steps.Add(hookStepTracker); + return hookStepTracker; } } \ No newline at end of file diff --git a/Reqnroll/Formatters/ExecutionTracking/TestStepExecutionTracker.cs b/Reqnroll/Formatters/ExecutionTracking/TestStepExecutionTracker.cs index 7cbcd0f73..34be4d482 100644 --- a/Reqnroll/Formatters/ExecutionTracking/TestStepExecutionTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/TestStepExecutionTracker.cs @@ -17,22 +17,23 @@ public async Task ProcessEvent(StepStartedEvent stepStartedEvent) { StepStartedAt = stepStartedEvent.Timestamp; - // if this is the first time to execute this step for this test, generate the properties needed to Generate the TestStep Message (stored in a TestStepTracker) - if (ParentTracker.IsFirstAttempt) - { - TestCaseTracker.ProcessEvent(stepStartedEvent); - } + // Resolve (or, on first sight across all attempts, create) the ledger entry for this step. + // We cannot trust AttemptCount here: an earlier attempt may have aborted before reaching this + // step, so the entry might not yet exist even though this is not the first attempt. + var pickleStepId = stepStartedEvent.StepContext.StepInfo.PickleStepId; + var occurrence = ParentTracker.NextOccurrence(StepKind.TestStep, pickleStepId); + StepTracker = TestCaseTracker.GetOrCreateTestStepTracker(pickleStepId, occurrence); - StepTracker = TestCaseTracker.GetTestStepTrackerByPickleId(stepStartedEvent.StepContext.StepInfo.PickleStepId); await Publisher.PublishAsync(Envelope.Create(MessageFactory.ToTestStepStarted(this))); } public async Task ProcessEvent(StepFinishedEvent stepFinishedEvent) { - if (ParentTracker.IsFirstAttempt) + // Reuse the ledger entry resolved at StepStarted; capture of binding details is idempotent + // (guarded inside TestStepTracker) so re-execution on a retry does not duplicate it. + if (StepTracker is TestStepTracker testStepTracker) { - var testStepTracker = TestCaseTracker.GetTestStepTrackerByPickleId(stepFinishedEvent.StepContext.StepInfo.PickleStepId); - testStepTracker?.ProcessEvent(stepFinishedEvent); + testStepTracker.ProcessEvent(stepFinishedEvent); } StepFinishedAt = stepFinishedEvent.Timestamp; diff --git a/Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs b/Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs index 3aed638ae..4445cc8d7 100644 --- a/Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs @@ -10,12 +10,16 @@ namespace Reqnroll.Formatters.ExecutionTracking; /// Tracks the information needed for a Cucumber Messages "test case step", that is a step with binding information. /// The test case step needs to be built upon the first execution attempt of a pickle. /// -public class TestStepTracker(string testStepId, string pickleStepId, TestCaseTracker parentTracker) - : StepTrackerBase(testStepId) +public class TestStepTracker(string testStepId, string pickleStepId, int occurrence, TestCaseTracker parentTracker) + : StepTrackerBase(testStepId, occurrence) { public TestCaseTracker ParentTracker { get; } = parentTracker; public string PickleStepId { get; } = pickleStepId; + // Guards the one-time capture of binding details. The same ledger entry is reused across + // retry attempts; capturing more than once would append duplicate argument lists. + private bool _bindingDetailsCaptured; + // Indicates whether the step was successfully bound to a Step Definition. public bool IsBound { get; private set; } // The Step Definition(s) that match this step of the Test Case. None for no match, 1 for a successful match, 2 or more for Ambiguous match. @@ -27,13 +31,14 @@ public class TestStepTracker(string testStepId, string pickleStepId, TestCaseTra public bool IsAmbiguous { get; private set; } - public void ProcessEvent(StepStartedEvent stepStartedEvent) - { - } - - // Once the StepFinishedAt event fires, we can finally capture which step binding was used and the arguments sent as parameters to the binding method + // Once the StepFinishedAt event fires, we can finally capture which step binding was used and the arguments sent as parameters to the binding method. + // This is idempotent: only the first attempt that runs the step to completion captures the binding details. public void ProcessEvent(StepFinishedEvent stepFinishedEvent) { + if (_bindingDetailsCaptured) + return; + _bindingDetailsCaptured = true; + DetectBindingStatus(stepFinishedEvent, out var isBound, out bool isAmbiguous, out List stepDefinitionIds, out var bindingMatches); IsBound = isBound; IsAmbiguous = isAmbiguous; diff --git a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/HookStepExecutionTrackerTests.cs b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/HookStepExecutionTrackerTests.cs index 80f542fc5..de0a59ee8 100644 --- a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/HookStepExecutionTrackerTests.cs +++ b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/HookStepExecutionTrackerTests.cs @@ -84,7 +84,7 @@ public HookStepExecutionTrackerTests() } private TestCaseExecutionTracker CreateTestCaseExecutionRecord(int attemptId = 0) => - new(_testCaseTrackerMock.Object, attemptId, "testCaseStartedId", "testCaseId", _testCaseTracker, _messageFactoryMock.Object, _publisherMock.Object, _stepTrackerFactoryMock.Object); + new(_testCaseTrackerMock.Object, attemptId, "testCaseStartedId", "testCaseId", _messageFactoryMock.Object, _publisherMock.Object, _stepTrackerFactoryMock.Object); [Fact] public async Task HookStepTracker_ProcessEvent_HookBindingStartedEvent_PublishesOneEnvelope() @@ -234,7 +234,8 @@ private HookStepTracker CreateDummyHookStepDefinition(string hookId) { return new HookStepTracker( "dummyTestStepId", - hookId + hookId, + 1 ); } } diff --git a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/PickleExecutionTrackerTests.cs b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/PickleExecutionTrackerTests.cs index 394bd2687..caeb97755 100644 --- a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/PickleExecutionTrackerTests.cs +++ b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/PickleExecutionTrackerTests.cs @@ -345,6 +345,58 @@ public async Task ProcessEvent_OutputAddedEvent_CreatesOutputMessage() _mockMessageFactory.Verify(m => m.ToAttachment(It.IsAny()), Times.Once); } + [Fact] + public async Task ProcessEvent_TruncatedRetry_SecondAttemptReachesNewStep_NoExceptionAndCompleteLedger() + { + // Regression test: attempt 0 fails at step ps1 (StopAtFirstError); step ps2 is never reached. + // Attempt 1 (retry): ps1 passes, then ps2 runs for the first time. + // Before the fix, the lookup for ps2 on attempt 1 threw NullReferenceException because + // the ledger entry had never been created (it was gated behind AttemptCount == 0). + var tracker = CreatePickleExecTracker(); + + var publishedEnvelopes = new List(); + _mockPublisher + .Setup(p => p.PublishAsync(It.IsAny())) + .Callback(env => publishedEnvelopes.Add(env)) + .Returns(Task.CompletedTask); + + var step1Context = new Mock(); + step1Context.Setup(x => x.StepInfo).Returns(new StepInfo(StepDefinitionType.Given, "step 1", null, null, "ps1")); + + var step2Context = new Mock(); + step2Context.Setup(x => x.StepInfo).Returns(new StepInfo(StepDefinitionType.Given, "step 2", null, null, "ps2")); + + // Attempt 0: only ps1 fires (scenario is then aborted by StopAtFirstError) + await tracker.ProcessEvent(new ScenarioStartedEvent(_featureContextStub, _scenarioContextSub)); + await tracker.ProcessEvent(new StepStartedEvent(_featureContextStub, _scenarioContextSub, step1Context.Object)); + await tracker.ProcessEvent(new StepFinishedEvent(_featureContextStub, _scenarioContextSub, step1Context.Object)); + _scenarioContextSub.ScenarioExecutionStatus = ScenarioExecutionStatus.TestError; + await tracker.ProcessEvent(new ScenarioFinishedEvent(_featureContextStub, _scenarioContextSub)); + + // Attempt 1 (retry): ps1 passes, then ps2 fires for the first time + var act = async () => + { + await tracker.ProcessEvent(new ScenarioStartedEvent(_featureContextStub, _scenarioContextSub)); + await tracker.ProcessEvent(new StepStartedEvent(_featureContextStub, _scenarioContextSub, step1Context.Object)); + await tracker.ProcessEvent(new StepFinishedEvent(_featureContextStub, _scenarioContextSub, step1Context.Object)); + await tracker.ProcessEvent(new StepStartedEvent(_featureContextStub, _scenarioContextSub, step2Context.Object)); + await tracker.ProcessEvent(new StepFinishedEvent(_featureContextStub, _scenarioContextSub, step2Context.Object)); + _scenarioContextSub.ScenarioExecutionStatus = ScenarioExecutionStatus.OK; + await tracker.ProcessEvent(new ScenarioFinishedEvent(_featureContextStub, _scenarioContextSub)); + await tracker.FinalizeTracking(); + }; + + await act.Should().NotThrowAsync(); + + // Ledger must contain both steps: ps1 from attempt 0, ps2 discovered on attempt 1 + tracker.TestCaseTracker.Steps.OfType().Should().HaveCount(2); + + // TestCase (definition) message must be published exactly once, regardless of retry + publishedEnvelopes.Count(e => e.Content() is TestCase).Should().Be(1); + + _scenarioContextSub.ScenarioExecutionStatus = ScenarioExecutionStatus.OK; // reset shared context + } + private PickleExecutionTracker CreatePickleExecTracker() { return new PickleExecutionTracker( diff --git a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestCaseTrackerTests.cs b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestCaseTrackerTests.cs new file mode 100644 index 000000000..7e8af1fbc --- /dev/null +++ b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestCaseTrackerTests.cs @@ -0,0 +1,80 @@ +using System.Linq; +using FluentAssertions; +using Gherkin.CucumberMessages; +using Moq; +using Reqnroll.Formatters.ExecutionTracking; +using Xunit; + +namespace Reqnroll.RuntimeTests.Formatters.ExecutionTracking; + +public class TestCaseTrackerTests +{ + private readonly Mock _idGeneratorMock; + private readonly TestCaseTracker _sut; + private int _idCounter; + + public TestCaseTrackerTests() + { + _idCounter = 0; + _idGeneratorMock = new Mock(); + _idGeneratorMock.Setup(g => g.GetNewId()).Returns(() => $"id-{++_idCounter}"); + + var pickleTrackerMock = new Mock(); + pickleTrackerMock.SetupGet(p => p.IdGenerator).Returns(_idGeneratorMock.Object); + + _sut = new TestCaseTracker("testCaseId", "pickleId", pickleTrackerMock.Object); + } + + [Fact] + public void GetOrCreateTestStepTracker_SamePickleIdAndOccurrence_ReturnsSameInstance() + { + // Simulates the same step being looked up on attempt 0 then again on attempt 1. + // Both attempts produce occurrence=1 for ps1, so the same ledger entry is reused. + var first = _sut.GetOrCreateTestStepTracker("ps1", 1); + var second = _sut.GetOrCreateTestStepTracker("ps1", 1); + + first.Should().BeSameAs(second); + _sut.Steps.Should().HaveCount(1); + } + + [Fact] + public void GetOrCreateTestStepTracker_TruncatedAttempt0_StepOnlySeenOnRetry_AppendsNewEntry() + { + // Attempt 0 is truncated: only ps1 fires (occurrence=1). ps2 is never reached. + var ps1Attempt0 = _sut.GetOrCreateTestStepTracker("ps1", 1); + + // Attempt 1 (retry): ps1 fires again (finds existing entry), then ps2 fires for the first time. + var ps1Retry = _sut.GetOrCreateTestStepTracker("ps1", 1); + var ps2Retry = _sut.GetOrCreateTestStepTracker("ps2", 1); + + ps1Attempt0.Should().BeSameAs(ps1Retry); + ps2Retry.Should().NotBeSameAs(ps1Attempt0); + _sut.Steps.Should().HaveCount(2); + _sut.Steps.OfType().Should().Contain(t => t.PickleStepId == "ps2"); + } + + [Fact] + public void GetOrCreateHookStepTracker_SameHookFiredMultipleTimes_CreatesOneEntryPerOccurrence() + { + // BeforeStep fires once per step; three steps yield three distinct occurrences of the same hook. + var occ1 = _sut.GetOrCreateHookStepTracker("hook1", 1); + var occ2 = _sut.GetOrCreateHookStepTracker("hook1", 2); + var occ3 = _sut.GetOrCreateHookStepTracker("hook1", 3); + + occ1.Should().NotBeSameAs(occ2); + occ2.Should().NotBeSameAs(occ3); + _sut.Steps.Should().HaveCount(3); + } + + [Fact] + public void GetOrCreateHookStepTracker_SameOccurrenceOnRetry_ReturnsSameInstance() + { + // Simulates the same hook firing as occurrence=1 on attempt 0 and again on attempt 1. + // The ledger entry must be stable across retries. + var first = _sut.GetOrCreateHookStepTracker("hook1", 1); + var second = _sut.GetOrCreateHookStepTracker("hook1", 1); + + first.Should().BeSameAs(second); + _sut.Steps.Should().HaveCount(1); + } +} diff --git a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestStepExecutionTrackerTests.cs b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestStepExecutionTrackerTests.cs index b6f2512d0..fa0907245 100644 --- a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestStepExecutionTrackerTests.cs +++ b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestStepExecutionTrackerTests.cs @@ -37,6 +37,7 @@ public TestStepExecutionTrackerTests() _testCaseExecutionTrackerStub = CreateTestCaseExecutionTracker(); _pickleExecutionTrackerMock.SetupGet(t => t.TestCaseTracker).Returns(_testCaseTrackerStub); + _pickleExecutionTrackerMock.SetupGet(t => t.IdGenerator).Returns(_idGeneratorMock.Object); // Setup mocked objects for tests _testStepExecutionTrackerSut = new TestStepExecutionTracker( @@ -48,7 +49,7 @@ public TestStepExecutionTrackerTests() } private TestCaseExecutionTracker CreateTestCaseExecutionTracker(int attemptId = 0) => - new(_pickleExecutionTrackerMock.Object, attemptId, "testCaseStartedId", "testCaseId", _testCaseTrackerStub, _messageFactoryMock.Object, _publisherMock.Object, _stepTrackerFactoryMock.Object); + new(_pickleExecutionTrackerMock.Object, attemptId, "testCaseStartedId", "testCaseId", _messageFactoryMock.Object, _publisherMock.Object, _stepTrackerFactoryMock.Object); [Fact] public async Task TestStepTracker_GenerateFrom_StepStartedEvent_PublishesOneEnvelope() @@ -152,7 +153,7 @@ public async Task TestStepTracker_ProcessEvent_StepFinishedEvent_FirstAttempt_Po var stepFinishedEvent = new StepFinishedEvent(null, scenarioContextMock.Object, stepContextMock.Object); - var definitionStub = new TestStepTracker("stepId", "stepPickleId", null); + var definitionStub = new TestStepTracker("stepId", "stepPickleId", 1, null); _testStepExecutionTrackerSut.StepTracker = definitionStub; _pickleExecutionTrackerMock.SetupGet(t => t.AttemptCount).Returns(0); // First attempt @@ -198,7 +199,7 @@ public async Task TestStepTracker_ProcessEvent_StepFinishedEvent_UndefinedStep_P .Setup(f => f.ToSuggestion(_testStepExecutionTrackerSut, It.IsAny(), It.IsAny(), It.IsAny())) .Returns(suggestion); - var definitionStub = new TestStepTracker("testStepId", "undefinedPickleId", null); + var definitionStub = new TestStepTracker("testStepId", "undefinedPickleId", 1, null); _testStepExecutionTrackerSut.StepTracker = definitionStub; _pickleExecutionTrackerMock.SetupGet(t => t.AttemptCount).Returns(0); diff --git a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestStepTrackerTests.cs b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestStepTrackerTests.cs index 3362ffe12..1ead5e282 100644 --- a/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestStepTrackerTests.cs +++ b/Tests/Reqnroll.RuntimeTests/Formatters/ExecutionTracking/TestStepTrackerTests.cs @@ -55,13 +55,13 @@ public void PopulateStepDefinitionFromExecutionResult_Should_Set_Bound_And_Argum stepContextMock.SetupGet(x => x.StepInfo).Returns(stepInfo); stepContextMock.SetupGet(x => x.Status).Returns(ScenarioExecutionStatus.OK); - _testCaseTracker.Steps.Add(new TestStepTracker("stepDefId", "stepPickleId", _testCaseTracker)); + _testCaseTracker.Steps.Add(new TestStepTracker("stepDefId", "stepPickleId", 1, _testCaseTracker)); _pickleExecutionTracker.StepDefinitionsByBinding = new ReadOnlyDictionary( new Dictionary { { stepBindingMock.Object, "stepPickleId" } }); var evt = new StepFinishedEvent(null, null, stepContextMock.Object); - var def = new TestStepTracker("stepDefId", "stepPickleId", _testCaseTracker); + var def = new TestStepTracker("stepDefId", "stepPickleId", 1, _testCaseTracker); // Act def.ProcessEvent(evt); @@ -103,13 +103,13 @@ text argument used in a step. stepContextMock.SetupGet(x => x.StepInfo).Returns(stepInfo); stepContextMock.SetupGet(x => x.Status).Returns(ScenarioExecutionStatus.OK); - _testCaseTracker.Steps.Add(new TestStepTracker("stepDefId", "stepPickleId", _testCaseTracker)); + _testCaseTracker.Steps.Add(new TestStepTracker("stepDefId", "stepPickleId", 1, _testCaseTracker)); _pickleExecutionTracker.StepDefinitionsByBinding = new ReadOnlyDictionary( new Dictionary { { stepBindingMock.Object, "stepPickleId" } }); var evt = new StepFinishedEvent(null, null, stepContextMock.Object); - var def = new TestStepTracker("stepDefId", "stepPickleId", _testCaseTracker); + var def = new TestStepTracker("stepDefId", "stepPickleId", 1, _testCaseTracker); // Act def.ProcessEvent(evt); @@ -152,7 +152,7 @@ public void PopulateStepDefinitionFromExecutionResult_Should_Set_Bound_False_Whe var evt = new StepFinishedEvent(null, null, stepContextMock.Object); - var def = new TestStepTracker("stepDefId", "pickleStepId", _testCaseTracker); + var def = new TestStepTracker("stepDefId", "pickleStepId", 1, _testCaseTracker); // Act def.ProcessEvent(evt); @@ -208,13 +208,13 @@ public void PopulateStepDefinitionFromExecutionResult_Should_Set_Ambiguous_When_ //_messageFactoryMock.Setup(f => f.CanonicalizeStepDefinitionPattern(It.IsAny())) // .Returns("ambiguousPattern"); - _testCaseTracker.Steps.Add(new TestStepTracker("ambiguousId", "pickleStepId", _testCaseTracker)); + _testCaseTracker.Steps.Add(new TestStepTracker("ambiguousId", "pickleStepId", 1, _testCaseTracker)); _pickleExecutionTracker.StepDefinitionsByBinding = new ReadOnlyDictionary( new Dictionary { { stepBindingMock.Object, "pickleStepId" } }); var evt = new StepFinishedEvent(null, scenarioContextMock.Object, stepContextMock.Object); - var def = new TestStepTracker("stepDefId", "pickleStepId", _testCaseTracker); + var def = new TestStepTracker("stepDefId", "pickleStepId", 1, _testCaseTracker); // Act def.ProcessEvent(evt); From 1a06befb292c395aba2e037614107ac18c3b5171 Mon Sep 17 00:00:00 2001 From: Chris Rudolphi <1702962+clrudolphi@users.noreply.github.com> Date: Wed, 24 Jun 2026 09:16:00 -0500 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- CHANGELOG.md | 2 +- Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a008e5fc..a96edfbe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ * Updated Cucumber dependencies to: Gherkin v39.1.0, Cucumber.Messages v32.0.1 and Cucumber.HtmlFormatter v23.1.0. Formatters.Tests modified by adopting use of Cucumber/CCK (v29.2.2). (#984) ## Bug fixes: -* Fix: A scenario wtih a Retry mechanism (such as NUnitRetry.ReqnrollPlugin) and with 'StopAtFirstError' enabled, can cause a null reference exception in the Cucumber Formatters. (#1083) +* Fix: A scenario with a Retry mechanism (such as NUnitRetry.ReqnrollPlugin) and with 'StopAtFirstError' enabled, can cause a null reference exception in the Cucumber Formatters. (#1083) * Fix: GenerateFeatureFileCodeBehindTask fails with misleading DirectoryNotFoundException when ndjson output path exceeds Windows MAX_PATH (260) *Contributors of this release (in alphabetical order):* @clrudolphi diff --git a/Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs b/Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs index 4445cc8d7..6d33dddf6 100644 --- a/Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/TestStepTracker.cs @@ -37,9 +37,9 @@ public void ProcessEvent(StepFinishedEvent stepFinishedEvent) { if (_bindingDetailsCaptured) return; - _bindingDetailsCaptured = true; DetectBindingStatus(stepFinishedEvent, out var isBound, out bool isAmbiguous, out List stepDefinitionIds, out var bindingMatches); + _bindingDetailsCaptured = true; IsBound = isBound; IsAmbiguous = isAmbiguous; StepDefinitionIds = stepDefinitionIds; From d2cdc40e7c6728228c639b1da647ccb871e9949b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 24 Jun 2026 14:17:40 +0000 Subject: [PATCH 3/3] Update HookStepTracker XML doc comment to reflect lazy ledger creation --- Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs b/Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs index 504a16d83..f7aacad86 100644 --- a/Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs +++ b/Reqnroll/Formatters/ExecutionTracking/HookStepTracker.cs @@ -2,7 +2,7 @@ /// /// Tracks the information needed for a Cucumber Messages "hook step", that is a hook with binding information. -/// The hook step needs to be built upon the first execution attempt of a pickle. +/// Hook step ledger entries are created lazily across execution attempts and are keyed by (hookId, occurrence). /// public class HookStepTracker(string testStepId, string hookId, int occurrence) : StepTrackerBase(testStepId, occurrence) {