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

Fix -var handling #34

Merged
merged 1 commit into from
Jul 23, 2020
Merged

Fix -var handling #34

merged 1 commit into from
Jul 23, 2020

Conversation

paultyng
Copy link
Contributor

@paultyng paultyng commented Jul 19, 2020

-var values were passed incorrectly as args, this cleans it up and adds testing in an apply test by roundtripping a variable to an output.

I also added a helper to simplify the exec.Cmd assertions logic (including env var checking, as this will be useful for reattach and some other things). I added t.Run in places where there were multiple sequential test cases to give cleaner output and scoping but also to allow for testing both runs even if the first fails.

This is dependent upon #32

@paultyng paultyng added the bug Something isn't working label Jul 19, 2020
@paultyng paultyng added this to the v0.4.0 milestone Jul 19, 2020
@paultyng paultyng force-pushed the logging branch 5 times, most recently from 2e5e74b to 21c9bef Compare July 20, 2020 14:20
Base automatically changed from logging to master July 20, 2020 17:40
@paultyng paultyng requested a review from kmoe July 20, 2020 17:45
@paultyng paultyng marked this pull request as ready for review July 20, 2020 17:45
Add apply test with -var usage, simplify exec.Cmd assertions
Copy link
Member

@kmoe kmoe 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!

@paultyng paultyng merged commit eae914d into master Jul 23, 2020
@paultyng paultyng deleted the fixvar branch July 23, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants