diff options
author | Florian Klink <flokli@flokli.de> | 2023-05-18T19·09+0300 |
---|---|---|
committer | flokli <flokli@flokli.de> | 2023-05-18T19·38+0000 |
commit | e31008db14054cc884ec2f8f4f7f5a07872fa398 (patch) | |
tree | 2ebf14cd41ea63546b46931325fd572a2c846ff5 | |
parent | 103d2eb90936e7897c20a9db96782aab5b9334e1 (diff) |
fix(tvix/store): fix timing sensitivity in gRPC directorysvc test r/6166
One test in here assumed the server was fast, but when very busy, a computer running these tests might not be fast. Treat both cases that can occur separately. Change-Id: Iba168ad39f2458c4fb8873211df33beeaff7c6c1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/8595 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
-rw-r--r-- | tvix/store/src/directoryservice/grpc.rs | 69 |
1 files changed, 45 insertions, 24 deletions
diff --git a/tvix/store/src/directoryservice/grpc.rs b/tvix/store/src/directoryservice/grpc.rs index 3c76d9e315b5..d4ac9fd925fd 100644 --- a/tvix/store/src/directoryservice/grpc.rs +++ b/tvix/store/src/directoryservice/grpc.rs @@ -434,6 +434,14 @@ mod tests { .put(DIRECTORY_B.clone()) .expect_err("must fail"); + // Putting DIRECTORY_B in a put_multiple will succeed, but the close + // will always fail. + { + let mut handle = directory_service.put_multiple_start(); + handle.put(DIRECTORY_B.clone()).expect("must succeed"); + handle.close().expect_err("must fail"); + } + // Uploading A and then B should succeed, and closing should return the digest of B. let mut handle = directory_service.put_multiple_start(); handle.put(DIRECTORY_A.clone()).expect("must succeed"); @@ -458,36 +466,49 @@ mod tests { .expect("must succeed") ); - // Uploading B and then A should fail during close (if we're a - // fast client) - let mut handle = directory_service.put_multiple_start(); - handle.put(DIRECTORY_B.clone()).expect("must succeed"); - handle.put(DIRECTORY_A.clone()).expect("must succeed"); - handle.close().expect_err("must fail"); + // Uploading B and then A should fail, because B refers to A, which + // hasn't been uploaded yet. + // However, the client can burst, so we might not have received the + // error back from the server. + { + let mut handle = directory_service.put_multiple_start(); + // sending out B will always be fine + handle.put(DIRECTORY_B.clone()).expect("must succeed"); + + // whether we will be able to put A as well depends on whether we + // already received the error about B. + if handle.put(DIRECTORY_A.clone()).is_ok() { + // If we didn't, and this was Ok(_), … + // a subsequent close MUST fail (because it waits for the + // server) + handle.close().expect_err("must fail"); + } + } - // Below test is a bit timing sensitive. We send B (which refers to - // A, so should fail), and wait sufficiently enough for the server + // Now we do the same test as before, send B, then A, but wait + // sufficiently enough for the server to have s // to close us the stream, // and then assert that uploading anything else via the handle will fail. - - let mut handle = directory_service.put_multiple_start(); - handle.put(DIRECTORY_B.clone()).expect("must succeed"); - - let mut is_closed = false; - for _try in 1..20 { - if handle.is_closed() { - is_closed = true; - break; + { + let mut handle = directory_service.put_multiple_start(); + handle.put(DIRECTORY_B.clone()).expect("must succeed"); + + let mut is_closed = false; + for _try in 1..1000 { + if handle.is_closed() { + is_closed = true; + break; + } + std::thread::sleep(time::Duration::from_millis(10)) } - std::thread::sleep(time::Duration::from_millis(200)) - } - assert!( - is_closed, - "expected channel to eventually close, but never happened" - ); + assert!( + is_closed, + "expected channel to eventually close, but never happened" + ); - handle.put(DIRECTORY_A.clone()).expect_err("must fail"); + handle.put(DIRECTORY_A.clone()).expect_err("must fail"); + } }); tester_runtime.block_on(task)?; |