Fix #1083 NullReferenceException in CucumberMessageFactory.ToTestStep…

.gitignore

@@ -381,3 +381,4 @@ nCrunchTemp*.csproj
 
 *.received.*
 
+/.claude

CHANGELOG.md

@@ -4,6 +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: GenerateFeatureFileCodeBehindTask fails with misleading DirectoryNotFoundException when ndjson output path exceeds Windows MAX_PATH (260)
 
 *Contributors of this release (in alphabetical order):* @clrudolphi

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)));
     }

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.
 /// </summary>
-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;
 }

Reqnroll/Formatters/ExecutionTracking/ITestCaseExecutionTrackerFactory.cs

@@ -8,6 +8,5 @@ TestCaseExecutionTracker CreateTestCaseExecutionTracker(
         IPickleExecutionTracker parentTracker,
         int attemptId,
         string testCaseId,
-        TestCaseTracker testCaseTracker,
         IMessagePublisher picklePublisher);
 }

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)

Reqnroll/Formatters/ExecutionTracking/StepKind.cs

@@ -0,0 +1,10 @@
+namespace Reqnroll.Formatters.ExecutionTracking;
+
+/// <summary>
+/// Discriminates the two kinds of tracked test-case steps for identity/occurrence keying.
+/// </summary>
+public enum StepKind
+{
+    TestStep,
+    Hook
+}

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;
+
+    /// <summary>
+    /// The 1-based index of this step/hook among occurrences of the <i>same identity</i>
+    /// (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.
+    /// </summary>
+    public int Occurrence { get; } = occurrence;
 }

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<StepExecutionTrackerBase> _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;
     }
 
+    /// <summary>
+    /// Returns the next 1-based occurrence index for the given step/hook identity within this attempt.
+    /// Called exactly once per StepStarted / HookBindingStarted event.
+    /// </summary>
+    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;
 

Reqnroll/Formatters/ExecutionTracking/TestCaseExecutionTrackerFactory.cs

@@ -10,15 +10,13 @@ public TestCaseExecutionTracker CreateTestCaseExecutionTracker(
         IPickleExecutionTracker parentTracker,
         int attemptId,
         string testCaseId,
-        TestCaseTracker testCaseTracker,
         IMessagePublisher picklePublisher = null)
     {
         return new TestCaseExecutionTracker(
             parentTracker,
             attemptId,
             idGenerator.GetNewId(),
             testCaseId,
-            testCaseTracker,
             messageFactory,
             picklePublisher ?? publisher,
             stepTrackerFactory);

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;
 /// <summary>
 /// 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.
 /// </summary>
 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)
+    /// <summary>
+    /// Returns the ledger entry for the <paramref name="occurrence"/>-th execution of pickle step
+    /// <paramref name="pickleStepId"/> 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).
+    /// </summary>
+    public TestStepTracker GetOrCreateTestStepTracker(string pickleStepId, int occurrence)
     {
-        return Steps.OfType<TestStepTracker>().FirstOrDefault(sd => sd.PickleStepId == pickleId);
-    }
+        var existing = Steps.OfType<TestStepTracker>()
+            .FirstOrDefault(sd => sd.PickleStepId == pickleStepId && sd.Occurrence == occurrence);
+        if (existing != null)
+            return existing;
 
-    public HookStepTracker GetHookStepTrackerByHookId(string hookId)
-    {
-        return Steps.OfType<HookStepTracker>().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)
+    /// <summary>
+    /// Hook counterpart of <see cref="GetOrCreateTestStepTracker"/>, keyed on (<paramref name="hookId"/>, <paramref name="occurrence"/>).
+    /// 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.
+    /// </summary>
+    public HookStepTracker GetOrCreateHookStepTracker(string hookId, int occurrence)
     {
-        var hookId = ParentTracker.StepDefinitionsByBinding[hookBindingStartedEvent.HookBinding];
+        var existing = Steps.OfType<HookStepTracker>()
+            .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;
     }
 }

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;

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.
 /// </summary>
-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<string> stepDefinitionIds, out var bindingMatches);
         IsBound = isBound;
         IsAmbiguous = isAmbiguous;

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
         );
     }
 }