Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

Non-existing import path with less side effects #240

Closed
wants to merge 4 commits into from

Conversation

sdboyer
Copy link
Member

@sdboyer sdboyer commented Feb 13, 2017

Using a github.com repository for a fixture intended to not exist has the unfortunate side effect of, at least sometimes, cauisng the tool to hang when prompting the user for a password. Hopefully this'll be improved by using a valid, deducible import path pointing to a nonexistent domain instead.

Fixes #214 (I hope)

Using a github.com repository for a fixture intended to not exist has
the unfortunate side effect of, at least sometimes, cauisng the tool to
hang when prompting the user for a password. Hopefully this'll be
improved by using a valid, deducible import path pointing to a
nonexistent domain instead.
@sdboyer sdboyer force-pushed the friendlier-notexist branch from c45a0b6 to 271a7d7 Compare February 13, 2017 13:53
@sdboyer
Copy link
Member Author

sdboyer commented Feb 13, 2017

Yeah, this doesn't seem to be making it any better 😡

@tro3
Copy link
Contributor

tro3 commented Feb 13, 2017

Seems like the issue isn't so much the password request, but that the goroutine is not collected before the test moves on. So it's a race to see if the test can finish before that thread cleans up. The password request certainly forces the outcome, but the race is still on without one.

@sdboyer
Copy link
Member Author

sdboyer commented Feb 13, 2017

@tro3 absolutely true. I'm just trying this as a stopgap, as indicated on #214 (though not reiterated here).

@tro3
Copy link
Contributor

tro3 commented Feb 13, 2017

I'll try to stare at it and see if I can help, but it is down in the weeds in an architecture I haven't wrapped my head around. Are you comfortable enough with the gps test suite that if I find a fix that doesn't break anything else, it is likely to fly?

@sdboyer
Copy link
Member Author

sdboyer commented Feb 13, 2017

I appreciate it, but yeah, it's very far down in the weeds, and while the solver itself is quite well-covered with tests, the source manager is less so. If you've got the time to look at things, it's probably not the ideal place to spend it 😄

@sdboyer
Copy link
Member Author

sdboyer commented Feb 21, 2017

This seems to wholly miss the problem, so I'm just going to close it.

@sdboyer sdboyer closed this Feb 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration tests failing?
3 participants