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

Merge in changes from @asnowfix fork #17

Merged
merged 132 commits into from
Dec 4, 2019
Merged

Merge in changes from @asnowfix fork #17

merged 132 commits into from
Dec 4, 2019

Conversation

ringods
Copy link
Contributor

@ringods ringods commented Nov 30, 2019

Merge in changes from the fork of @asnowfix. Let us first stabilize this branch via this PR.

TODO:

  • Make unit tests run
  • Fix linting issues
  • Activate Github Actions
  • Remove TravisCI & CircleCI setup

@ringods
Copy link
Contributor Author

ringods commented Nov 30, 2019

Hello @asnowfix, with this PR I am pulling in all the changes which reside in your fork of this repo. The merge commit however still has some problems. I'm not so much a Go developer so I need some input to get the unit test running. On our setup, we were already on Go 1.12, so I assume that the failure has to do with this. Could you have a look?

$ make test
ME=ringods
INTERACTIVE=1
PKGS=github.com/terra-farm/go-virtualbox github.com/terra-farm/go-virtualbox/cmd/vbhostd
go test -v ./...
panic: testing: Verbose called before Init

goroutine 1 [running]:
testing.Verbose(...)
        /usr/local/Cellar/go/1.13.4/libexec/src/testing/testing.go:389
github.com/terra-farm/go-virtualbox.logF(0x118c369, 0x11, 0x0, 0x0, 0x0)
        /Users/ringods/Projects/terra-farm/go-virtualbox/log_test.go:19 +0xe5
github.com/terra-farm/go-virtualbox.init.0()
        /Users/ringods/Projects/terra-farm/go-virtualbox/log_test.go:28 +0x6d
FAIL    github.com/terra-farm/go-virtualbox     0.241s
?       github.com/terra-farm/go-virtualbox/cmd/vbhostd [no test files]
FAIL
make: *** [test] Error 1

@ringods
Copy link
Contributor Author

ringods commented Nov 30, 2019

Only after I submitted the request for help, I noticed that the tests failed on my laptop but worked on TravisCI. The difference was the Go version. My laptop was running Go 1.13 and TravisCI 1.12. So I added Go 1.13 to the Travis setup to verify the failure I noticed on my laptop.

So a search around the webs for golang 1.13 testing rendered me the following issue:

golang/go#31859

In another issue, I found an example workaround:

flipt-io/flipt@cd7fb49#diff-77b87a2786da4b38e7dd9d4fd463953c

Commit 44041a6 contains a similar fix which now also makes the tests work on Go 1.13.

@ringods
Copy link
Contributor Author

ringods commented Dec 3, 2019

@VoyTechnology do you have some time to review? Please let me know. If you don't have time, I will go forward as-is with merging this.

@ringods ringods merged commit ba34deb into master Dec 4, 2019
@ringods
Copy link
Contributor Author

ringods commented Dec 4, 2019

Going forward with this.

@ringods ringods deleted the merge-in-asnowfix branch December 18, 2019 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants