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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions helper/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,9 +459,10 @@ func (m schemaMap) diffList(
o, n, _, computedList := d.diffChange(k)
nSet := n != nil

// If we have an old value, but no new value set but we're computed,
// then nothing has changed.
if o != nil && n == nil && schema.Computed {
// If we have an old value and no new value is set or will be
// computed once all variables can be interpolated and we're
// computed, then nothing has changed.
if o != nil && n == nil && !computedList && schema.Computed {
return nil
}

Expand Down
51 changes: 51 additions & 0 deletions helper/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,57 @@ func TestSchemaMap_Diff(t *testing.T) {

Err: false,
},

{
Schema: map[string]*Schema{
"internal": &Schema{
Type: TypeBool,
Required: true,
},

"instances": &Schema{
Type: TypeSet,
Elem: &Schema{Type: TypeString},
Optional: true,
Computed: true,
Set: func(v interface{}) int {
return len(v)
},
},
},

State: &terraform.InstanceState{
Attributes: map[string]string{
"internal": "false",
"instances.#": "0",
},
},

Config: map[string]interface{}{
"internal": true,
"instances": []interface{}{"${var.foo}"},
},

ConfigVariables: map[string]string{
"var.foo": config.UnknownVariableValue,
},

Diff: &terraform.InstanceDiff{
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...

},

"instances.#": &terraform.ResourceAttrDiff{
Old: "0",
NewComputed: true,
},
},
},

Err: false,
},
}

for i, tc := range cases {
Expand Down