Fix intermittent test problem in rivertest + more transactional use
#226
Conversation
14babd3 to
6c66096
Compare
This one's aimed at fixing #225, in which I think what was happening is that because we were inserting a test job on a non-transactional pool with a state of `scheduled`, in some cases the queue maintainer's scheduler service was actually changing it back to `available` before the test case would check it with a `RequireInserted`. As I was moving through the test suite, I realized that we could be making a lot more use of `InsertTx` variants pretty much everywhere since we were mostly testing the transactional variables of `RequireInserted`, so that I changed those over. Then I realize that even when we were testing the non-transactional variants, we didn't actually need a started client to do so, so I took that code out. Then I realized that since we're not starting the client, we didn't actually need to specify `Queues` or `Workers` (I believe the idea of an insert-only came about only after I wrote these tests originally). And if we didn't need to specify `Workers`, then we didn't even need the worker structs, so I got rid of those too. Overall, I think we simplify things quite a bit, and because the client doesn't need to be started, probably reduces the possibility of future intermittency problems like we observed in #225.
6c66096 to
b4cd117
Compare
Yep, all of this correct. Primarily the fix here is to not run maintenance processes for tests that don't need or want them. In the course of investigating #213, I've also realized we have the scheduler running frequently in most tests, which is another thing we don't need most of the time and probably adds up to a fair bit of overhead. I'm sure there are other similar issues & inefficiencies throughout the tests to clear out. |
|
Great, thanks! |
This one's aimed at fixing #225, in which I think what was happening is
that because we were inserting a test job on a non-transactional pool
with a state of
scheduled, in some cases the queue maintainer'sscheduler service was actually changing it back to
availablebeforethe test case would check it with a
RequireInserted.As I was moving through the test suite, I realized that we could be
making a lot more use of
InsertTxvariants pretty much everywheresince we were mostly testing the transactional variables of
RequireInserted, so that I changed those over.Then I realize that even when we were testing the non-transactional
variants, we didn't actually need a started client to do so, so I took
that code out. Then I realized that since we're not starting the client,
we didn't actually need to specify
QueuesorWorkers(I believe theidea of an insert-only came about only after I wrote these tests
originally). And if we didn't need to specify
Workers, then we didn'teven need the worker structs, so I got rid of those too.
Overall, I think we simplify things quite a bit, and because the client
doesn't need to be started, probably reduces the possibility of future
intermittency problems like we observed in #225.
Fixes #225.