Skip to content

Add regression test for chunked ResendRequest boundaries and queue drain#1156

Draft
Copilot wants to merge 2 commits intomasterfrom
copilot/add-session-test-chunked-resend
Draft

Add regression test for chunked ResendRequest boundaries and queue drain#1156
Copilot wants to merge 2 commits intomasterfrom
copilot/add-session-test-chunked-resend

Conversation

Copy link
Contributor

Copilot AI commented Mar 6, 2026

Adds a regression test covering the end-to-end flow where incoming application messages arrive far ahead of the expected target sequence number on a FIX 4.2 session with chunked resend requests, verifying that chunk boundaries are correct and that queued messages are drained and delivered to fromApp() once the gap is filled.

Changes

New test: testChunkedResendRequestBoundariesAndQueueDrain_FIX42_chunk2500_fromAppCalledForQueuedAppMessages

  • FIX42 acceptor session, resendRequestChunkSize=2500, expected seqnum advanced to 57989
  • Receives 13 app messages (61989–62001), all queued; verifies none are delivered to fromApp() before the gap is filled
  • Asserts first resend chunk: beginSeqNo=57989, currentEndSeqNo=60488
  • After SequenceReset GapFill(57989→60489): asserts second chunk beginSeqNo=60489, currentEndSeqNo=61988
  • After final SequenceReset GapFill(60489→61989): asserts queue empty and fromApp() called for all 13 queued seqnums

Supporting additions

  • createFix42AppMessage(int) — FIX42-specific News message helper; required because generated message classes embed their protocol's BeginString in the header and Session.next() validates it against the session version, making FIX44 helpers incompatible with FIX42 sessions
  • CountingApplication — minimal Application implementation tracking fromAppCount and fromAppSeqNums
  • java.util.Set / java.util.HashSet imports

Design note

The final GapFill uses NewSeqNo=61989 (the first queued message) rather than a value past all queued messages. Using a higher NewSeqNo would cause dequeueMessagesUpTo() to silently discard the queued application messages before nextQueued() has a chance to deliver them.

Original prompt

Create a pull request in quickfix-j/quickfixj that adds a new unit test to quickfixj-core/src/test/java/quickfix/SessionTest.java.

Goal

Add a regression/unit test that simulates receiving application messages with sequence numbers far ahead of the expected target sequence number, triggering chunked resend requests and queueing of those messages, then satisfying the resend ranges with SequenceReset-GapFill messages, and finally verifying that the queued messages are drained and delivered to the application callback.

Requirements

  1. Add new test method named exactly:

    • testChunkedResendRequestBoundariesAndQueueDrain_FIX42_chunk2500_fromAppCalledForQueuedAppMessages
  2. The test must:

    • Use a SessionID with FixVersions.BEGINSTRING_FIX42.
    • Configure resend chunk size to 2500 by constructing the Session directly and passing resendRequestChunkSize = 2500.
    • Set session.setNextTargetMsgSeqNum(57989).
    • Logon (use existing helper logonTo(session, 1)).
    • Receive queued application messages with MsgSeqNum 61989, 61990, 61991 and then 61992..62001.
      • IMPORTANT: these incoming messages must be FIX 4.2 News messages (quickfix.fix42.News) created via a new helper createFix42AppMessage(int).
    • Assert that before the gap is filled, none of the queued messages have been delivered to Application.fromApp().
    • Capture resend request chunk boundaries by adding a SessionStateListener and recording calls to onResendRequestSent(SessionID, beginSeqNo, endSeqNo, currentEndSeqNo).
      • Assert the first chunk has begin=57989 and currentEnd=60488.
      • After processing a SequenceReset GapFill createSequenceReset(57989, 60489, true), assert a second chunk has begin=60489 and currentEnd=61988.
    • After processing final SequenceReset GapFill createSequenceReset(60489, 62002, true):
      • Assert the in-memory queue is empty (queue.getBackingMap().isEmpty()).
      • Assert fromApp() was called for all queued message seqnums 61989..62001 using a single unified loop.
  3. Add a minimal Application implementation inside SessionTest:

    • Class name: CountingApplication
    • Implements quickfix.Application
    • Records:
      • int fromAppCount
      • Set<Integer> fromAppSeqNums
    • fromApp(...) increments fromAppCount and records the message header MsgSeqNum.
    • Other callbacks can be no-ops.
  4. Add helper method inside SessionTest:

    • private quickfix.fix42.News createFix42AppMessage(int sequence)
    • Similar to existing createAppMessage(int) but returns FIX42 News.
  5. Remove any mention of the string event.txt in names/comments of the new code.

Notes

  • Keep the test consistent with existing patterns in SessionTest (use existing helper methods such as getSessionState, processMessage, createSequenceReset, logonTo, etc.).
  • Ensure the new code compiles: add imports if needed, or fully qualify types where appropriate.
  • Do not modify unrelated tests.

This pull request was created from Copilot chat.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…romAppCalledForQueuedAppMessages and supporting code

Co-authored-by: chrjohn <6644028+chrjohn@users.noreply.github.com>
Copilot AI changed the title [WIP] Add unit test for chunked resend request and queue drain Add regression test for chunked ResendRequest boundaries and queue drain in FIX42 sessions Mar 6, 2026
@chrjohn chrjohn changed the title Add regression test for chunked ResendRequest boundaries and queue drain in FIX42 sessions Add regression test for chunked ResendRequest boundaries and queue drain Mar 7, 2026
@chrjohn
Copy link
Member

chrjohn commented Mar 7, 2026

There is a race condition but I still need to think about how this can be shown in unit test. After setting the next expected seqnum there is a chance that the queue of messages is emptied before the messages get read via nextQueued().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants