-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sleep statement in tests with VCR enabled #20916
Sleep statement in tests with VCR enabled #20916
Comments
Yeah, it would make sense to skip the sleep if we're in REPLAYING mode. I don't think that would cause any issues |
I guess my questions were a) whether it makes sense to have a way to do that built in to the function, and also whether this might apply to other uses of it in the codebase? Or would you suggest just putting a conditional in the check function for this case only? |
I only see 4 uses of this function in our tests and for 10 seconds or less each time. So not a big deal. If we want to fix this I'd add the check for REPLAYING within this function to skip the sleep in that case |
In `SleepInSecondsForTest()`, skip the sleep when `VCR_MODE` is `REPLAYING`. This makes the assumption that the sleep is always related to a condition that's already mitigated by the fact that we're using prerecorded fixtures, but it could be extended later to override this behavior if necessary. Fixes hashicorp/terraform-provider-google#20916
@slevenick let me know if the PR I raised is what you had in mind. I just wanted to check first since I don't totally know what decision making was behind those other ~ 4 sleeps in the codebase. |
In `SleepInSecondsForTest()`, skip the sleep when `VCR_MODE` is `REPLAYING`. This makes the assumption that the sleep is always related to a condition that's already mitigated by the fact that we're using prerecorded fixtures, but it could be extended later to override this behavior if necessary. Fixes hashicorp/terraform-provider-google#20916
In `SleepInSecondsForTest()`, skip the sleep when `VCR_MODE` is `REPLAYING`. This makes the assumption that the sleep is always related to a condition that's already mitigated by the fact that we're using prerecorded fixtures, but it could be extended later to override this behavior if necessary. Fixes hashicorp/terraform-provider-google#20916
Question
It seems like SleepInSecondsForTest() is enabled for some tests that have VCR enabled. Should there be default or configurable behavior to skip the sleep if
IsVcrEnabled()
isTrue
? I would imagine most cases where sleeps are needed in tests are related to cases in recording vs. replaying mode?The text was updated successfully, but these errors were encountered: