-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Make VCR correctly handle compound tests #13941
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
Conversation
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR hasn't generated any diffs, but I'll let you know if a future commit does. |
|
@modular-magician reassign-reviewer |
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @slevenick, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
|
@modular-magician reassign-reviewer |
Tests analyticsTotal tests: 23 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
/gcbrun |
Tests analyticsTotal tests: 23 Click here to see the affected service packages
🟢 All tests passed! View the build log |
|
@slevenick This PR is approved and has been waiting for merge for 1 week. Is it ready to merge? Use the label |
Tests analyticsTotal tests: 3 Click here to see the affected service packages
View the build log |
|
@slevenick This PR is approved and has been waiting for merge for 2 weeks. Is it ready to merge? Use the label |
Tests analyticsTotal tests: 3 Click here to see the affected service packages
View the build log |
This reverts commit 900c2ad.
|
@slevenick This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
Error message for the failing test is giving me a 404, is that related to this change? |
Ah, good catch. It looks like the build logs for compound tests use the compound test name while the debug logs use the subtest name. There is probably a more elegant solution to this problem but it would require changing the Result struct around. |
Tests analyticsTotal tests: 3 Click here to see the affected service packages
Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
🟢 Tests passed during RECORDING mode: 🟢 No issues found for passed tests after REPLAYING rerun. 🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
|
@GoogleCloudPlatform/terraform-team @slevenick This PR has been waiting for review for 1 week. Please take a look! Use the label |
This reverts commit 1b1705d.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR hasn't generated any diffs, but I'll let you know if a future commit does. |
c4aaf26
fixes hashicorp/terraform-provider-google#19930
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.