Improved tests for vmpooler #152
Conversation
Open questions: - Do we need to do better cleanup here? - Should we be using a separate database to prevent clobbering other local db's?
Miscellaneous whitespace cleanup.
Given the way we're flushing redis (which seems super performant), we don't need to clear pools any more at the beginning of tests.
The mockist version of the test allows redis' scard to return nil, which it does not actually do in real life. Verified the behavior in the code via a debugger. Fixed the test.
|
Could use some 👀 on this now. I've upgraded the API tests as well as the dashboard tests to use live redis and no longer rely on mocking and stubbing. This fixed a few tests in the process, and ends up testing the actual underlying state of the world in a few cases, where before we were mostly just checking that an HTTP status code with valid JSON were being returned. There are a couple of files I didn't touch (in part because they seemed weird and confusing, and in part to ship something of benefit, even if not everything is covered): The work from #153 (which was merged to the ci.next branch) would want to merge this up and have the new tests it introduced updated once this lands. Shouldn't be difficult. A couple of other cc's of potentially interested parties: @shermdog @heathseals |
|
I'm going to merge my own PR unless I get a 👎. |
The tests for the vmpooler mock and stub their way to glory. The actual code is really heavy on the "use crazy redis strings everywhere" (which is mirrored in the tests). Taking a stab at making these tests be non-mockist, using actual state on the back-end, and defining the semantics of the operations we're performing.
My hope is that we get a less fragile + more readable suite in the process, and also can begin to refactor the code base, to extract and put semantic names around bits of redis-munging code scattered around (also, move the redis code out of the API into the lower
lib/service layer(s)).Open issues / TODO:
/cc @stahnma @heathseals @joshcooper