test(terraform): add assertion for terraform stdout #6040
Conversation
Follow-up on #6037 (This adds a missing test)
| @@ -624,7 +624,7 @@ describe("getManifests", () => { | |||
| expect(manifests[0].spec.replicas).to.eql(3) | |||
| }) | |||
| it("should log a warning if patches don't match manifests", async () => { | |||
| garden.log.root["entries"] = [] | |||
| garden.log.root["entries"].length = 0 | |||
There was a problem hiding this comment.
Is this a more preferred way of emptying arrays? I'm just curious :)
There was a problem hiding this comment.
I did this as changing the identity of the private array made the test fail in the terraform tests (I didn't quite understand why). Setting length to 0 removes all elements of the array but preserves it's identity, and to prevent others from experiencing the same thing I changed all occurrences of this "reset" code to the more reliable method preserving the identity.
There was a problem hiding this comment.
It think it'd be less hacky if we added a "reset" method to the logger interface or implemented a helper somewhere 🤔
There was a problem hiding this comment.
Thanks for the explanation! I've just looked to the official JS reference docs and it looks like it's quite a common way of emptying an array :)
So, no need to create a helper)
There was a problem hiding this comment.
If we need to mutate an array without reassigning it and changing its identity, that's the way to go :)
The official docs say that the exceed elements will be removed.
That can be understood as those won't be kept by any internal reference and will become available for garbage collection :)
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Follow-up on #6037 (This adds a missing test)