Skip to content
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

provider/google: Fix Pubsub acceptance tests #5439

Merged
merged 1 commit into from
Mar 3, 2016

Conversation

evandbrown
Copy link
Contributor

Acceptance tests for Pubsub topics and subscriptions failed after
incorrectly determining that resources were not deleted in the
CheckDestroy phase.

Fixes #5437

Acceptance tests for Pubsub topics and subscriptions failed after
incorrectly determining that resources were not deleted in the
CheckDestroy phase.

Fixes 5437
@evandbrown
Copy link
Contributor Author

The CheckDestroy func was interpreting a non-nil error from the Get{Topic,Subscription} calls to mean the topic still existed, when the opposite is true. I removed checking err in favor of checking the first return value (the resource type) as it's easier to read.

What's thoroughly confused me is why this test run passed: https://travis-ci.org/hashicorp/terraform/jobs/108197224

@evandbrown
Copy link
Contributor Author

In fact, PubSub tests continued to pass after the test I linked above. This is the test where they started to fail. Any thoughts, @lwander?

@phinze
Copy link
Contributor

phinze commented Mar 3, 2016

That test behavior is definitely odd, but these changes look good so I'm going to pull them in so they make the nightly run for tonight. Here's to a hopefully passing suite! 🍻

phinze added a commit that referenced this pull request Mar 3, 2016
provider/google: Fix Pubsub acceptance tests
@phinze phinze merged commit 8d31c93 into hashicorp:master Mar 3, 2016
@lwander
Copy link
Contributor

lwander commented Mar 3, 2016

Here's why: 3eb65f2#diff-5f8e56f07503aa8fcc49a3e77a381794R39

The code wasn't returning the error, and that commit from 19 days ago fixed that, but inadvertently broke the test.

Thanks for getting these fixed, @evandbrown!

terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
terraformbot pushed a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
jen20 added a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
jen20 added a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
jen20 added a commit that referenced this pull request Mar 3, 2016
[origin/master] Merge pull request #5439 from evandbrown/pubsub
8d31c93
@ghost
Copy link

ghost commented Apr 27, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TestAccPubsubTopicCreate and TestAccPubsubSubscriptionCreate fail
3 participants