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

Set up test cases where ProjectRoot decls in manifests/locks are not actual roots #434

Closed
fabulous-gopher opened this issue Apr 21, 2017 · 5 comments

Comments

@fabulous-gopher
Copy link

From @sdboyer on October 14, 2016 14:3

This is an important case - users will no doubt bump into it when getting used to gps-based tooling, and probably inadvertently down the road. It needs sane failure modes.

Copied from original issue: sdboyer/gps#109

@sdboyer
Copy link
Member

sdboyer commented Apr 26, 2017

Some more info here...

Right now, the solver just sorta blithely assumes that all the incoming Manifest.Dependencies() from the root (actually, all, I think) are, themselves, project roots. We need tests that cover situations where that's not actually true, and ensure that we error out appropriately.

This might be a bit of a rabbit hole, as gps' solver testing harness isn't generally designed to accommodate bad inputs like this. As such, it may or may not be possible to construct a pure declarative test case that gets at this behavior. But that would be the place to start - declaring e.g. b/subpkg as a root in a constraint, when the actual project root is just b.

If that doesn't work, we may need a more custom-tailored test, like TestRootLockNoVersionPairMatching, to hit this behavior.

Additionally, TestBadSolveOpts should get an entry for this class of failure, as that's the phase where the check should happen - at least for the root.

We do also need to test the case where a dependency specifies a non-root, but that's probably a case for a warning rather than an overall failure, so we can handle it separately.

@cmlicata
Copy link

@sdboyer, I'd like to take this on, but will wait until after the transition of moving gps into dep is complete. Sound Good?

@sdboyer
Copy link
Member

sdboyer commented Apr 27, 2017

@cmlicata sure, of course. FWIW, if you wanted to start digging now on gps in its original standalone form, it would still be productive - nothing's changing during the move that would affect this. but definitely wait on a PR until after we've got it merged in 😄

@mikijov
Copy link
Contributor

mikijov commented Jun 19, 2017

Hi @cmlicata. Just wanted to check if you are still looking into this. I am looking for an easy issue to dig into, and this caught my eye.

@cmlicata
Copy link

cmlicata commented Jun 19, 2017 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants