Reorganize dummy downstairs tests #1253
Conversation
| // TODO is this correct? Shouldn't values in the mpsc queue be read out, | ||
| // even if the sender has been dropped? |
There was a problem hiding this comment.
I don't think our ClientIoTask drains the requests before stopping: a quick glace at client.rs and it looks like landing in a self.stop branch breaks out of the task?
There was a problem hiding this comment.
That makes sense, I forgot that the ClientIoTask drops values while exiting.
There was a problem hiding this comment.
(removed the comment in a64ce0c)
| @@ -2342,6 +2342,10 @@ pub(crate) enum ClientStopReason { | |||
|
|
|||
| /// The upstairs has requested that we deactivate | |||
| Deactivated, | |||
|
|
|||
| /// The test suite has requested a fault | |||
| #[cfg(test)] | |||
There was a problem hiding this comment.
I don't know how it would work, but it might be nice to have a way to expose this interface
as a build option? We could write various tests that would trigger a fault during specific
times. I suppose it's not much different than just stopping or pstopping a downstairs process
which is our go to method now.
| "could not send read response for job {} = {}: {}", | ||
| i, | ||
| job_id, | ||
| "could not send read response for job {}: {}", |
There was a problem hiding this comment.
This is an expected situation, right? If so, we might want to change the log message to indicate it is expected?
There was a problem hiding this comment.
Sure, done in 1cc5719
mkeeter
force-pushed
the
reorganize-dummy-ds-tests
branch
from
a64ce0c to
1cc5719
Compare
This is more prep for addressing #1252
Previously, our live-repair tests were checking two different things in a single test:
There's also a separate test,
test_byte_fault_condition(#1192), to confirm that sending > 1 GiB caused a Downstairs to be marked as faulted.This PR splits those two checks into separate unit tests, adding new (test-only) APIs to (1) check Downstairs state and (2) manually fault a Downstairs. We want these checks to be separate because – once we make the changes for #1252 – sending 57K jobs will no longer cause an active downstairs to be marked as faulted.
In addition, common
TestHarnesscode is consolidated into helper functions:Guestandtokio::spawn→TestHarness::spawnDownstairsHandle::ack_read,ack_write,ack_flush