terraform icon indicating copy to clipboard operation
terraform copied to clipboard

Enhancements for comparing data structures in test results

Open wyardley opened this issue 6 months ago • 5 comments

Terraform Version

Terraform v1.12.0
on darwin_arm64

Use Cases

The changes in #36522 (for #34428) look great and are working really well. One thought is that when comparing structures that are generated via jsonencode(), the escaping makes them pretty hard to read. Maybe this is too niche a problem to solve, but would it be possible to diff the json directly in the case where we're comparing jsonencoded data on both sides?

In either case, with an error that the LHS and RHS are different types, would it be possible to display the internal type of each?

Attempted Solutions

  assert {
    # This messy jsonencode is necessary for now; see:
    # https://github.com/hashicorp/terraform/issues/34428
    condition = jsonencode(google_secret_manager_secret.this.labels) == jsonencode({
      "foo" = "asdfadsf", ## actually "bar"
      "baz" = "qux",
    })
    error_message = "Expected members not found."
  }

This behaves correctly, and is not a bug, however, when the structures being compared are json and the results don't match, the structures are compared with quoting and escaped double quotes, which makes them difficult to read.

│     │ --- actual
│     │ +++ expected
│     │ - "{\"baz\":\"qux\",\"foo\":\"bar\"}"
│     │ + "{\"baz\":\"qux\",\"foo\":\"asdfadsf\"}"
│ 
Image

(side note: not sure if +++ expected should also be green to match?)

Directly comparing the two without jsonencode() still doesn't work when the values are the expected values and in the expected order, though it does now produce a more useful error:

  assert {
    condition = google_secret_manager_secret.this.labels == {
      "baz" = "qux",
      "foo" = "bar",
    }
    error_message = "Expected members not found."
  }
│ Error: Test assertion failed
│ 
│   on tests/main.tftest.hcl line 88, in run "simple_example":
│   88:     condition = google_secret_manager_secret.this.labels == {
│   89:       "baz" = "qux",
│   90:       "foo" = "bar",
│   91:     }
│     ├────────────────
│     │ LHS:
│     │   {
│     │     "baz": "qux",
│     │     "foo": "bar"
│     │   }
│     │ RHS:
│     │   {
│     │     "baz": "qux",
│     │     "foo": "bar"
│     │   }
│     │ Warning: LHS and RHS values are of different types
│ 
│     │ google_secret_manager_secret.this.labels is {
│     │     "baz": "qux",
│     │     "foo": "bar"
│     │   }

In this case, it would be nice if we could list the type of the LHS and RHS explicitly, which I think it still does not do? (I can make a separate issue if requested). It's also interesting that the visual representation of each is both (though from the error, it seems like they're different types internally).

Presumably actually being able to compare these two directly would be a different / separate enhancement, and IIRC there may already even be an issue open about it.

Proposal

│   on tests/main.tftest.hcl line 90, in run "simple_example":
│   90:     condition = jsonencode(google_secret_manager_secret.this.labels) == jsonencode({
│   91:       "foo" = "asdfadsf",
│   92:       "baz" = "qux",
│   93:     })
│     ├────────────────
│     │ Diff:
│     │ --- actual
│     │ +++ expected
│     │ - {"baz": "qux", "foo": "bar"}
│     │ + {"baz": "qux", "foo": "asdfadsf"}

References

No response

wyardley avatar May 16 '25 17:05 wyardley

Thanks for this detailed feature request, @wyardley!

If you are viewing this issue and would like to indicate your interest, please use the 👍 reaction on the issue description to upvote this issue. We also welcome additional use case descriptions.

Thanks again!

crw avatar May 16 '25 18:05 crw

Hi @wyardley,

For a quick reason as to why this fails, the value google_secret_manager_secret.this.labels is defined as a map(string) while the literal value in the config is a object({foo=string,baz=string}), so the comparison will always fail because the types don't match. In this particular case, the comparison could be made valid by wrapping the object literal in tomap(), but that does require more detailed knowledge of the resource schema.

The fact that the output is a quoted string is because that is all the language sees at the point of comparison. The jsonencode function returns a string and has already been evaluated by that point, so there is no direct context to somehow generate a different error message. Guessing that the user wanted a structural difference of strings which look like json would probably lead to issues filed in the reverse, where the expectation is that a strings are only ever compared as strings. This is already reaching into the language internals a bit to hijack the default error message, so I'm inclined to start investing other options. Perhaps assertions can natively have the ability to do comparisons with type conversion and generate their own diffs, rather than relying on simple language equality.

jbardin avatar May 16 '25 19:05 jbardin

@jbardin thanks, and thanks for the quick response. Yeah, I suspected it was a different type internally (and also that the jsonencode() thing might be too niche to "fix" the rendering of).

That said, having it print the type when mentioning that types don't match, at least, could maybe be helpful (at least in debug or verbose mode?)

I'll see if there's an existing issue about comparisons. It looks like #35760, which kind of touched on this, got folded into #34428, which only really resolved the diagnostics part, but not the bit about having some sort of deep equality or similar function for doing object comparison.

I don't know what the name would be for being able to do some automagic type conversion or how hard that would be, but definitely that would be ideal IMO, and happy to make a separate request for that if you think that makes sense. I would guess that map < - > object or list of maps < - > list of objects would be the main cases where this would at all make sense?

wyardley avatar May 16 '25 19:05 wyardley

And probably obvious from the comment, but in this case, at least, I'm mostly only using jsonencode() to work around the lack of other ways to compare these two types.

wyardley avatar May 16 '25 19:05 wyardley

This issue is fine to track whatever improvements we may be able to come up with. I wouldn't limit conversions to only object/map if we're going to implement something that doesn't take a simple bool expression using an equality operator. I imagine instead of an HCL expression we would need to teach the assert code how to do various comparisons in order to generate a diff suitable for the context of a test assertion. Maybe something inspired by the go-cmp package which has customizable comparators that generate a diff on their own for testing.

jbardin avatar May 16 '25 19:05 jbardin