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 testing suites outside of the current directory tree with Go 1.8 and test against Go 1.8 in CI #337

Closed
wants to merge 4 commits into from

Conversation

goonzoid
Copy link

This was a fun one to figure out! 🤡

Ginkgo's own integration suite was failing when using go 1.8. It turns out that a change in Go 1.8 means that relative import paths outside of the current directory tree can't begin with ./.

Fix this by adding some logic to detect relative paths outside of the current directory tree, and then took the liberty of adding Go 1.8 to .travis.yml.

@goonzoid
Copy link
Author

And of course it fails on CI 😢

I'll take a closer look tomorrow or later this week and see if I can figure out what's different there compared to my local machine (may well be an OS X vs Linux thing I suppose).

This fixes an issue when using Go 1.8 where testing suites with relative
paths outside of the current directory tree would fail.
@goonzoid
Copy link
Author

So, turns out the integration suite is significantly slower using Go 1.8 ☹️

I bumped the default Eventually timeout in the integration suite to 30s (from 15s) and it's green now. Not sure if that's the right thing, or if it would be better to dig in to the perf issues. If that's preferred, I can back out the timeout change and the commit that adds Go 1.8 testing to CI and that can be handled separately.

Let me know! 🍔

@onsi
Copy link
Owner

onsi commented Mar 21, 2017 via email

@goonzoid
Copy link
Author

I've reverted the timeout increase and pushed a change without the -i flag just to see what happens on CI. I'm getting confusing and inconsistent results on my dev machine, but I'd like to dig in to it more in a day or two when I have some more time.

@goonzoid
Copy link
Author

@onsi looks like you've already figured this out on master, so I'll close this.

@goonzoid goonzoid closed this Apr 12, 2017
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.

2 participants