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

core: fixing a small logic bug in diffList #661

Merged
merged 1 commit into from
Dec 12, 2014
Merged

core: fixing a small logic bug in diffList #661

merged 1 commit into from
Dec 12, 2014

Conversation

svanharmelen
Copy link
Contributor

It’s not enough to only check if no new value is set. It can also be that a new value is set, but contains a variable that cannot be interpolated until a depending resource is created during the apply fase.

I actually found this one as one of the acceptance tests for the AWS ELB resource was failing. It failed with the following error:

--- FAIL: TestAccAWSELB_InstanceAttaching (177.83 seconds)
  testing.go:121: Step 1 error: Error applying: aws_elb.bar: diffs didn't match during apply. This is a bug with the resource provider, please report a bug.
FAIL
exit status 1
FAIL  github.com/hashicorp/terraform/builtin/providers/aws  177.882s

After a quick look I noticed it was actually a bug in core TF so added the test and made sure all unit tests and AWS acceptance tests are now running successfully.

It’s not enough to only check if no new value is set. It can also be
that a new value is set, but contains a variable that cannot be
interpolated until a depending resource is created during the apply
fase.

I actually found this one as one of the acceptance tests for the AWS
ELB resource was failing. It failed with the following error:

```
--- FAIL: TestAccAWSELB_InstanceAttaching (177.83 seconds)
  testing.go:121: Step 1 error: Error applying: aws_elb.bar: diffs
didn't match during apply. This is a bug with the resource provider,
please report a bug.
FAIL
exit status 1
FAIL  github.com/hashicorp/terraform/builtin/providers/aws  177.882s
```

After a quick look I noticed it was actually a bug in core TF so added
the test and made sure all unit tests and AWS acceptance tests are now
running successfully.
svanharmelen pushed a commit that referenced this pull request Dec 12, 2014
core: fixing a small logic bug in diffList
@svanharmelen svanharmelen merged commit db640cb into hashicorp:master Dec 12, 2014
@svanharmelen svanharmelen deleted the f-fix-difflist-logic-error branch December 12, 2014 14:31
Attributes: map[string]*terraform.ResourceAttrDiff{
"internal": &terraform.ResourceAttrDiff{
Old: "0",
New: "1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually affect the test? It might be simpler to just remove the "internal" part.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your right... The 'internal' part can be omitted. In the PR for changing the set behaviour I add two more tests to this file, so will update this test in that same PR...

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.

2 participants