-
Notifications
You must be signed in to change notification settings - Fork 87
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
Rename _vagrant to vagrant #67
Conversation
b6e9216
to
f8c3bfb
Compare
test/_vagrant/vagrant_test.go
Outdated
@@ -1,208 +0,0 @@ | |||
package vagrant_test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did you forget to commit the new dir?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did I?! Maybe!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!! :D Thanks
Apparently the idea to prefix a package with an underscore is not that smart as I thought. Yes `go test` does not run it by default when you run `go test ./...` but also other commands like `go mod tidy` do not work consistently. Nothing changes in practice. By default only unit tests run. Setting the new environment variable: `TEST_WITH_VAGRANT` you include the test who uses vagrant. Signed-off-by: Gianluca Arbezzano <[email protected]>
f8c3bfb
to
c5dd65c
Compare
@@ -16,6 +17,11 @@ import ( | |||
) | |||
|
|||
func TestVagrantSetupGuide(t *testing.T) { | |||
_, ok := os.LookupEnv("TEST_WITH_VAGRANT") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can also add a build constraint right? I believe that works for test files too. Not sure if thats any better than this env look up though tbh.
@gianarb I imagine you want to run the vagrant action in this PR too right? |
yes but for some reason the runner is not up, so I am looking at it first. But yes! I will merge only when vagrant on packet workflow is happy as well |
Hmm I added the label to see if GH will then block the PR until the runner is back and tests pass. |
Yep looks like it does, now you can't hit merge by mistake :p |
Also curious to see how this ends up failing. |
I am merting it manually because Mergify can't merge PRs who changes workflows |
Rename _vagrant to vagrant
Apparently the idea to prefix a package with an underscore is not that
smart as I thought. Yes
go test
does not run it by default when yourun
go test ./...
but also other commands likego mod tidy
do notwork consistently.
Nothing changes in practice. By default only unit tests run. Setting the
new environment variable:
TEST_WITH_VAGRANT
you include the test whouses vagrant.