Improve tests for multi threaded state store committer
- Dominant language
- Java
- Stars
- 107
- Forks
- 29
- Avg merge
- 19h 46m
- Merged PRs (30d)
- 141
Description
### User Story
As a user of Sleeper, I want changes to the Sleeper table state to be applied quickly and reliably, so that my data is not lost and I can retrieve the data I expect in a timely manner.
As a developer working on Sleeper, I want the implemented behaviours to have comprehensive test automation, so that I know if I've broken a behaviour, and which one I've broken.
As a developer working on Sleeper, I want tests to run as quickly as possible, so that I have to wait as little as possible for feedback on my changes.
### Description / Background
Follows on from:
- https://github.com/gchq/sleeper/issues/5091
Raised based on PR comment, see notes.
Some tests were written as part of the above issue, but a lot of the behaviour is not covered.
We'd like to provide comprehensive test coverage for that code, with unit tests where possible.
### Technical Notes / Implementation Details
Currently MultiThreadedStateStoreCommitterLocalStackIT checks the queue message count estimates to decide when to stop the test. This means several of the tests take over 20 seconds, because the SQS wait time is hard coded at 20 seconds. We can adjust to stop each test as soon as no message is received from the queue, and lower or remove the SQS wait time during tests.
We can cover most of the behaviour in MultiThreadedStateStoreCommitter with unit tests. We'll need to add interfaces to the SQS code and in-memory alternatives to that part of the code. We could extract the logic into a separate class, and move that into the statestore-committer-core module.
There's some code in MultiThreadedStateStoreCommitter.runUntil where it catches an exception in a catch block, assigns it to a local variable and then re-throws it in the finally block attached to it. This is unnecessary because if you rethrow an exception from a catch block, the finally block is still executed before the exception is thrown. This was noted in the following PR comment:
- https://github.com/gchq/sleeper/pull/6189#discussion_r2705179239
There's some code in MultiThreadedStateStoreCommitter that was automatically reformatted in a way that makes it harder to read, in the following PR. It would be good to undo some of this or improve the resulting readability:
- https://github.com/gchq/sleeper/pull/6462/files#diff-7de6108db83e9a5841ecb74c5b8789e640c6f3811cc899938178f2fb79cccd42
### Dependencies / Blockers
Conflicts with:
- https://github.com/gchq/sleeper/issues/6438
- https://github.com/gchq/sleeper/issues/6457
Contributor guide
Assessment
This issue has not been assessed yet.