search

hashicorp / hcl2

Former temporary home for experimental new version of HCL
https://github.com/hashicorp/hcl
Mozilla Public License 2.0
375 stars 64 forks source link

Add test cases for multiline heredocs #124

Closed habnabit closed 5 years ago

habnabit commented 5 years ago

These tests pass, but they close what's effectively a gap in the test coverage wherein nothing was ensuring that the whitespace prefix trimming behavior still gave the same/correct output with blank lines in the middle.

I made these tests because nomad templates were behaving oddly, only to discover that nomad has a mix of both hcl1 and hcl2 in it! Fortunately, hcl2's behavior is correct, but it should stay that way.

apparentlymart commented 5 years ago

Thanks @habnabit! This is some great extra coverage for something that is easy to get wrong for sure.

We also have some other test cases that are intended to be used to test conformance for any possible future implementations of HCL (in other languages, perhaps). Although that test suite is far from complete right now, we do specifically have a heredoc test in there (under expressions) specifically because that behavior has been historically tricky. Would you mind adding some similar tests to that heredoc test case in specsuite so that future implementations can be verified too?

As mentioned in the readme, we have some code in the Go package tests to run those specsuite tests as part of a normal go test on package ./hcl/spectests, so you shouldn't need to do anything unusual to run them for testing.

If you don't have any more time to work on this then that's totally fine; I'm happy to make those updates myself when I get a chance, if not.

Thanks again!

habnabit commented 5 years ago

I'm pretty sure I did change what you're talking about under specsuite already? I might be missing something, but I think I did update both the golang tests and the specsuite. I based the changes I made off of what was changed in e8dbb16dbc7f9774afd808c19a0fd40d2347191f.

apparentlymart commented 5 years ago

Hi @habnabit! Sorry, on a second look I see that you did; I guess something prevented the full "files changed" page from loading on my first read, and so I only saw the expression_test.go changes. :thinking:

In that case, great! I will merge it now. Thanks again.