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 helpers #1682

Merged
merged 1 commit into from
Aug 4, 2021
Merged

Refactor test helpers #1682

merged 1 commit into from
Aug 4, 2021

Conversation

stefanprodan
Copy link
Member

Changes:

  • move test helpers to main
  • add support for inline golden values
  • add test for flux --version

- move test helpers to main
- add support for inline golden values
- add test for `flux --version`

Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

  • Thank you Stefan 🙏

@stefanprodan stefanprodan merged commit 3570fab into main Aug 4, 2021
@stefanprodan stefanprodan deleted the refactor-tests branch August 4, 2021 08:15
@allenporter
Copy link
Contributor

Thanks, I had a similar PR in mind with some follow up improvements.

@stefanprodan
Copy link
Member Author

One thing I forgot to do is to delete that init, tests should use func TestMain.

@stefanprodan
Copy link
Member Author

Oh one more thing, the test helpers are using ioutil this needs to go away, see #1658

@allenporter
Copy link
Contributor

allenporter commented Aug 4, 2021 via email

@allenporter
Copy link
Contributor

allenporter commented Aug 4, 2021

Moving to TestMain sounds good, will do. I'm assuming all tests that have times/dates in the golden files will need that setup -- that is a TestMain per test file. Is that right?

I also realize there could be side effects for changing the timezone across tests.

@allenporter
Copy link
Contributor

allenporter commented Aug 4, 2021

I realize now, TestMain can do setup and teardown to handle restoring any environment variables.
But also, TestMain seems to be a test-wide initialization, rather than a per-test file initialization, so not necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants