Skip to content

Conversation

@ansd
Copy link
Member

@ansd ansd commented Jan 5, 2026

Test case resends_lost_command of rabbit_fifo_int_SUITE succeeded without testing what it is supposed to test.
That's because the mock used the wrong arity.
Applying the following diff

-    meck:expect(ra, pipeline_command, fun (_, _, _) -> ok end),
+    meck:expect(ra, pipeline_command, fun (_, _, _, low) -> ok end),

made the test case then fail with:

rabbit_fifo_int_SUITE > tests > resends_lost_command
    #1. {error,
            {{badmatch,
                 {empty,
                     {state,
                         {cfg,
                             [{resends_lost_command,ct_rabbit@VQD7JFK37T}],
                             32,10000},
                         {resends_lost_command,ct_rabbit@VQD7JFK37T},
                         go,3,4,false,#{},
                         #{1 => {undefined,{e,2,msg2,{0,0}}},
                           2 => {undefined,{e,3,msg3,{0,0}}}},
                         #{},undefined,undefined}}},
             [{rabbit_fifo_int_SUITE,resends_lost_command,1,
                  [{file,"rabbit_fifo_int_SUITE.erl"},{line,315}]},
              {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1796}]},
              {test_server,run_test_case_eval1,6,
                  [{file,"test_server.erl"},{line,1305}]},
              {test_server,run_test_case_eval,9,
                  [{file,"test_server.erl"},{line,1237}]}]}}

The timer was never actually scheduled in rabbit_fifo_client. This commit therefore removes all the dead code.

Note that resend_all_pending/1 is nowadays executed upon leader changes. nodeup in the queue proc also causes resend_all_pending/1 to be executed even if the leader didn't change.

Test case `resends_lost_command` of `rabbit_fifo_int_SUITE` succeeded
without testing what it is supposed to test.
That's because the mock used the wrong arity.
Applying the following diff
```
-    meck:expect(ra, pipeline_command, fun (_, _, _) -> ok end),
+    meck:expect(ra, pipeline_command, fun (_, _, _, low) -> ok end),
```
made the test case then fail with:
```
rabbit_fifo_int_SUITE > tests > resends_lost_command
    #1. {error,
            {{badmatch,
                 {empty,
                     {state,
                         {cfg,
                             [{resends_lost_command,ct_rabbit@VQD7JFK37T}],
                             32,10000},
                         {resends_lost_command,ct_rabbit@VQD7JFK37T},
                         go,3,4,false,#{},
                         #{1 => {undefined,{e,2,msg2,{0,0}}},
                           2 => {undefined,{e,3,msg3,{0,0}}}},
                         #{},undefined,undefined}}},
             [{rabbit_fifo_int_SUITE,resends_lost_command,1,
                  [{file,"rabbit_fifo_int_SUITE.erl"},{line,315}]},
              {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1796}]},
              {test_server,run_test_case_eval1,6,
                  [{file,"test_server.erl"},{line,1305}]},
              {test_server,run_test_case_eval,9,
                  [{file,"test_server.erl"},{line,1237}]}]}}
```

The timer was never actually scheduled in `rabbit_fifo_client`.
This commit therefore removes all the dead code.

Note that `resend_all_pending/1` is nowadays executed upon leader changes.
`nodeup` in the queue proc also causes `resend_all_pending/1` to be executed
even if the leader didn't change.
@ansd ansd force-pushed the da-fifo-client-timer branch from dd2f9d7 to bdf2029 Compare January 5, 2026 11:01
@ansd ansd added this to the 4.3.0 milestone Jan 5, 2026
@ansd ansd marked this pull request as ready for review January 5, 2026 12:38
@ansd ansd requested a review from kjnilsson January 5, 2026 12:38
@ansd ansd merged commit 7b765da into main Jan 6, 2026
291 checks passed
@ansd ansd deleted the da-fifo-client-timer branch January 6, 2026 08:06
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