WIP: [NO-JIRA] KahaDBOffsetRecoveryListenerTest execution time improvements

[mattrpav]

replaces [#1555]

[jeanouii]

Can you explain why you put back the @before with send/receive of several thousands of messages?
Why is this needed?

[rmannibucau]

@mattrpav maybe check another time https://github.com/apache/activemq/pull/1555/files and fork @jeanouii PR to propose your change, copying his work doesn't look fair from outside.

Also the changes look just random (the message count, the before method is completely pointless etc) so maybe superseed this PR with this one #1555 rather than the opposite to make test suite better, no?

[mattrpav]

Can you explain why you put back the @before with send/receive of several thousands of messages? Why is this needed?

The comment describes the scenario -- the queue's sequence number needs to be pushed forward to ensure correctness of the tests.

// Send+Recv a odd number of messages beyond cache sizes
// to confirm the queue's sequence number gets pushed off

[mattrpav]

@mattrpav maybe check another time https://github.com/apache/activemq/pull/1555/files and fork @jeanouii PR to propose your change, copying his work doesn't look fair from outside.

Also the changes look just random (the message count, the before method is completely pointless etc) so maybe superseed this PR with this one #1555 rather than the opposite to make test suite better, no?

I am always mindful to ensure contributors get credit for their efforts. In this case, the initial change needs to be reverted for correctness. Using two commits to do that does not seem productive -- the applied usage of the IOHelper ( < 1 line of change) is the only change from @jeanouii original branch.

That being said, I'm happy to add a co-author credit in the commit comment if that warrants.

[jeanouii]

@mattrpav but the queue where it's pushed is not the same as the queue used in the tests themselves.
And the reality is that if you remove it all together, it still works, so either some asserts are missing for the tests or really this is not used.

Can you explain why you put back the @before with send/receive of several thousands of messages? Why is this needed?

The comment describes the scenario -- the queue's sequence number needs to be pushed forward to ensure correctness of the tests.

// Send+Recv a odd number of messages beyond cache sizes
// to confirm the queue's sequence number gets pushed off

[mattrpav]

Timings on my local laptop with nvme disk

[INFO] Running org.apache.activemq.store.kahadb.KahaDBOffsetRecoveryListenerTest
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 140.0 s -- in org.apache.activemq.store.kahadb.KahaDBOffsetRecoveryListenerTest

Update: Current CI/CD time:

[INFO] Running org.apache.activemq.store.kahadb.KahaDBOffsetRecoveryListenerTest
[INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 446.9 s -- in 

[rmannibucau]

The comment describes the scenario -- the queue's sequence number needs to be pushed forward to ensure correctness of the tests.

this is not because it is written that it is true, you changed the number and it still passes so it is likely wrong no - at least code doesn't require it strictly and if using internal length you broke it by changing it and fake the test passes - but it doesn't test anything anymore?

So either you "removed" virtually the test or it is useless, both case are wrong in current PR - my understanding is it is useless.