Drive hash / decrypt tests from Upstairs::apply #1065
Conversation
| @@ -3504,4 +3508,641 @@ pub(crate) mod test { | |||
| assert_eq!(c.state(), DsState::New); | |||
| } | |||
| } | |||
|
|
|||
| #[tokio::test] | |||
| async fn good_decryption() { | |||
There was a problem hiding this comment.
This test is called "good_decryption" but the comment on line 3527 says we
are going to fail decryption? which is it :)
|
|
||
| #[tokio::test] | ||
| async fn bad_hash_on_encrypted_read_panic() { | ||
| let mut up = make_encrypted_upstairs(); |
There was a problem hiding this comment.
Could we add a comment here on what makes this test different than the
bad_read_hash_makes_panic test that follows? I think this test is altering
the tag?
There was a problem hiding this comment.
Good catch, this test was misleading (it altered the tag, but also had a bad hash). I removed the tag changes, since those are tested elsewhere, and added a comment.
mkeeter
force-pushed
the
event-driven-read-tests
branch
from
675d488 to
397c630
Compare
|
This failed Edit: this is #1077 , so unrelated to this branch |
mkeeter
force-pushed
the
event-driven-read-tests
branch
2 times, most recently
from
ed1324f to
2397037
Compare
mkeeter
force-pushed
the
event-driven-read-tests
branch
from
2397037 to
6c4e859
Compare
This PR converts hash / decryption read tests to run through
Upstairs::apply, instead of using internalDownstairsAPIs.It is in preparation for moving decryption off-thread (and I've got a branch which does that in the wings), but this is a standalone building block that shouldn't be too controversial.
I also made some small changes to tests where relevant:
work_read_hash_mismatch_ackandwork_read_hash_mismatch_third_ackare removed, because jobs are now acked immediately (so the in-between point that those tests are checking doesn't exist any more)work_read_hash_mismatch_insidewasn't actually testing the case of hashes vecs with different lengths (despite the comment); it was changed to actually check that by returning reads including 1 vs 2 hashes.bad_decryption_means_panicwasn't actually checking decryption: theUpstairswas constructed without an encryption context, so it was failing at the hash match stage.In all of the tests that are expected to panic, we now catch the panic string and do a quick substring check. (This is how I found the issue with
bad_decryption_means_panic)