From 8af34495c20f36c7008f1b5c01c27edd604f1f72 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Fri, 29 Mar 2024 19:21:55 -0700 Subject: [PATCH] Fix semver hazard in S3 stalled stream protection test (#3541) Merging #3485 uncovered a semver hazard in the S3 stalled stream protection test due to a subtle (but intentional) change in grace period behavior for stalled streams. That semver hazard was going to cause the SDK release to fail, so #3485 was reverted, and this PR modifies the test so that it will pass with both versions of stalled stream protection as a temporary measure. Once this PR is merged and released, then the new stalled stream protection can be merged again. I tested this by updating the runtime-versioner in #3540 so that I could patch the currently released SDK with these test changes in place, with a local copy of smithy-rs that has the new stalled stream protection implementation. I ran all the tests across all the services in this configuration to verify this was the only hazard that will be hit. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .../s3/tests/stalled-stream-protection.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/aws/sdk/integration-tests/s3/tests/stalled-stream-protection.rs b/aws/sdk/integration-tests/s3/tests/stalled-stream-protection.rs index 25008a415e..af38678f70 100644 --- a/aws/sdk/integration-tests/s3/tests/stalled-stream-protection.rs +++ b/aws/sdk/integration-tests/s3/tests/stalled-stream-protection.rs @@ -229,8 +229,14 @@ async fn test_stalled_stream_protection_for_downloads_is_enabled_by_default() { err.to_string(), "minimum throughput was specified at 1 B/s, but throughput of 0 B/s was observed" ); - // 1s check interval + 5s grace period - assert_eq!(start.elapsed().as_secs(), 6); + // 5s grace period + // TODO(https://github.com/smithy-lang/smithy-rs/issues/3510): Currently comparing against 5 and 6 due to + // the behavior change in #3485. Once that feature/fix is released, this should be changed to only check for 5. + let elapsed_secs = start.elapsed().as_secs(); + assert!( + elapsed_secs == 5 || elapsed_secs == 6, + "elapsed secs should be 5 or 6, but was {elapsed_secs}" + ) } async fn start_faulty_download_server() -> (impl Future, SocketAddr) {