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

tests: only provide template values when used #3592

Merged
merged 3 commits into from
Feb 15, 2023
Merged

Conversation

hiddeco
Copy link
Member

@hiddeco hiddeco commented Feb 15, 2023

As otherwise the .golden values can not be automatically updated using
-update as documented in CONTRIBUTING.md.

Also ensure we do not use defer but rather t.Cleanup in tests, as
this will always be called even if e.g. t.Fatal abruptly stops the
test.

Also fixed some other minor things I ran into, and filled issues for the
things that are beyond reach of time I have available.

@hiddeco hiddeco added the area/ci CI related issues and pull requests label Feb 15, 2023
@hiddeco hiddeco force-pushed the fix-golden-files branch 4 times, most recently from ed5cc8a to 66cd09e Compare February 15, 2023 12:21
As otherwise the `.golden` values can not be automatically updated using
`-update` as documented in `CONTRIBUTING.md`.

Also ensure we do not use `defer` but rather `t.Cleanup` in tests, as
this will always be called even if e.g. `t.Fatal` absruptly stops the
test.

Signed-off-by: Hidde Beydals <hello@hidde.co>
@@ -171,8 +171,7 @@ spec:
if err != nil {
t.Fatal(err)
}

Copy link
Member Author

@hiddeco hiddeco Feb 15, 2023

Choose a reason for hiding this comment

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

There is more here that could be done, as using ./testdata/ for something temporary seems wrong.

I think we should instead use t.TempDir() as a target for the file that is been written. Which would prevent having to think about garbage collecting it later on.

This is however beyond the time I have available now, as I was merely trying to fix things I ran into while working on #3587.

Signed-off-by: Hidde Beydals <hello@hidde.co>
As the other version has a different signature, but exists for a
different build tag. Resulting in my IDE becoming absolutely confused
when I tried to enable both at the same time. Opted for "exec" because
this one shells out.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

LGTM!

@hiddeco hiddeco merged commit ae97286 into main Feb 15, 2023
@hiddeco hiddeco deleted the fix-golden-files branch February 15, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci CI related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants