makefile: run containers/image unit tests

[cyphar]

Because containers/image doesn't vendor anything, it has trouble running
its own tests (it depends on packages which may change in the future).
To make sure that containers/image is tested with our own vendored
packages, run its tests as part of our unit tests.

This isn't enabled globally for obvious reasons.

Signed-off-by: Aleksa Sarai [email protected]

[runcom]

👍 @mtrmac PTAL

[cyphar]

Ah, dammit. vndr removes _test* files. I'll add a patch to vndr to fix this...

EDIT: Here's the patch: LK4D4/vndr#23.

[runcom]

EDIT: Here's the patch: LK4D4/vndr#23.

is this per vendored project or an all-or-nothing test files?

[cyphar]

It's only done in the final stage of "cleaning". So if you only update one project at a time (which is what we do) then it will only "not clean" the project in question. If @LK4D4 wants, I can extend it so that it's a regexp for what projects to clean or not clean.

[cyphar]

The extension is in LK4D4/vndr#24. So you would run vndr -whitelist "github\.com/containers/image/.*" to update everything but leave containers/image alone. You can also specify multiple whitelists.

[mtrmac]

Conceptually this seems strange to me; if we want to run containers/image unit tests against skopeo vendored packages to test a containers/image property (that containers/image works against those dependencies), why would that not be a part of the containers/image repo, the way test-skopeo is also inside the containers/image repo to test a containers/image property (that containers/image does not break skopeo without us knowing)?

Would we completely stop running unit tests in containers/image CI and hope that skopeo’s CI will eventually notice failures? That seems very unappealing.

Given the existing “clone skopeo” script in test-skopeo, that would be conceptually a trivial GOPATH=$skopeo_path/vendor:$GOPATH go test ./... (I’m sure that the actual code would be 2–5× trickier, but still much simpler than patching vndr and using a special process to revendor c/image in skopeo, and …)

[erikh]

Maybe an integration suite that lives in a third repo would be best?

[mtrmac]

With containers/image#240, we are now running containers/image tests against vendored dependencies, so the original motivation for this PR seems to no longer apply. @cyphar , can this be closed?

Maybe an integration suite that lives in a third repo would be best?

What value would the independent repo bring? If we wanted to run containers/image tests against the skopeo vendored dependencies¹, we could copy them equally easily from the containers/image repo and from a third independent repo.

¹ OTOH we’ve been toying with the idea of having an independent repo carrying vendor.conf which would be shared between containers/image and skopeo; then the re-run of tests would be obviously redundant.

Either way, splitting stuff across repositories makes CI more complex—when changing a dependency, we would like to make a CI run of all the users of that dependency as well (like we are doing by make test-skopeo in containers/image). So, the more repos, the more cross-repo CI infrastructure, longer edit loops… and in the end we would want to update most of the repos in lockstep pretty much all of the time.

Or am I missing something essential about a separate test-only repo?

[erikh]

I just feel the revendoring/cross-testing juggle should probably be done in a separate repo is all, it seems like it would make build tooling significantly simpler.

…

On Mon, Feb 27, 2017 at 12:52 PM, Miloslav Trmač ***@***.***> wrote: With containers/image#240 <containers/image#240>, we are now running containers/image tests against vendored dependencies, so the original motivation for this PR seems to no longer apply. @cyphar <https://github.com/cyphar> , can this be closed? Maybe an integration suite that lives in a third repo would be best? What value would the independent repo bring? *If* we wanted to run containers/image tests against the skopeo vendored dependencies¹, we could copy them equally easily from the containers/image repo and from a third independent repo. ¹ OTOH we’ve been toying with the idea of having an independent repo carrying vendor.conf which would be shared between containers/image and skopeo; then the re-run of tests would be obviously redundant. Either way, splitting stuff across repositories makes CI more complex—when changing a dependency, we would like to make a CI run of all the users of that dependency as well (like we are doing by make test-skopeo in containers/image). So, the more repos, the more cross-repo CI infrastructure, longer edit loops… and in the end we would want to update most of the repos in lockstep pretty much all of the time. Or am I missing something essential about a separate test-only repo? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#308 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABJ6_gOBKA5ziqpPWrDRMaPkTHpqONcks5rgzd2gaJpZM4MEOIy> .

[rhatdan]

Is this pull request still something we are considering?

[mtrmac]

It still seems to me that doing this in the containers/image repo, as a part of an extension to the existing make test-skopeo, would be much simpler—if this is still needed after containers/image#240 .

[cyphar]

The problem is that containers/image doesn't vendor anything, so it can't run any of the tests. The reason for that is because Go's nested vendoring is broken-by-design.

[mtrmac]

I’m not sure I understand:

• containers/image itself does not vendor dependencies in the repo, but for the purpose of running tests it does vendor in the dependencies: https://github.com/containers/image/blob/master/Makefile#L30
• skopeo does vendor dependencies in the repo, so the make test-skopeo command in containers/image can check out skopeo including the dependencies, and it seems to me that it could also use skopeo’s vendor tree as $GOPATH (±some symlinks, I guess) to run the containers/image tests. Am I overlooking something which would make this impossible?

[rhatdan]

@cyphar This needs a rebase or needs to be closed.

[rhatdan]

@cyphar Still waiting to see if I should close. Will close in one week.

[cyphar]

I'm going to close this since it's not really progressed much. I agree with @erikh that the project testing needs to be restructured (in the past skopeo had some growing pains that were only detected because of umoci's test suite -- and my hope with this was that we could avoid this issue by testing containers/image properly) but am not sure how it should look.