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

refactor(test): use t.Setenv #13036

Merged
merged 1 commit into from May 14, 2024
Merged

Conversation

agilgur5
Copy link
Member

@agilgur5 agilgur5 commented May 11, 2024

Related to #12862 (comment), #12756 (comment)

Motivation

t.Setenv handles setting the env var back to what it was after

  • automates the defer + Unsetenv at the end
    • and is more correct -- it always sets it back to what it was, not just unsets it
    • also catches a few places where the defer was missed which could've resulted in some incorrect tests

Modifications

os.Setenv + os.Unsetenv -> t.Setenv

Verification

Existing tests should pass

- [`t.Setenv`](https://pkg.go.dev/testing#B.Setenv) handles setting the env var back to what it was after
  - automates the `defer` + `Unsetenv` at the end
    - and is more correct -- it always sets it back to what it was, not just unsets it
    - also catches a few places where the `defer` was missed which could've resulted in some incorrect tests

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 added the area/build Build or GithubAction/CI issues label May 11, 2024
@juliev0 juliev0 merged commit 9cacef3 into argoproj:main May 14, 2024
29 checks passed
@agilgur5 agilgur5 deleted the refactor-test-t-setenv branch May 14, 2024 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Build or GithubAction/CI issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants