Makefile: fix 'make test' #243
Conversation
giuseppe
force-pushed
the
fix-make-test
branch
from
8cf3dc9 to
3d87088
Compare
Makefile
Outdated
|
|
||
| test: deps | ||
| @go test $(BUILDFLAGS) -cover $(PACKAGES) | ||
| @go test $(BUILDFLAGS) -cover $(PACKAGES) ./... |
There was a problem hiding this comment.
What is this supposed to fix? What is the failure, and how does this improve on it?
Look at PACKAGES is computed: it is supposed to be equivalent to ./..., except that vendor/** is excluded. We don’t really want to run tests in the vendored packages, so I can’t see what difference this should make.
There was a problem hiding this comment.
not sure if it is the correct fix but make test doesn't run any test for me on master. This is what I get:
$ make test
? github.com/projectatomic/skopeo/vendor/github.com/containers/image [no test files]
There was a problem hiding this comment.
That… doesn’t make any sense to me. How can the list of packages tested contain only a /vendor/ package which has been explicitly filtered out?
Can you look into how $PACKAGES is computed, and perhaps compare that with make --just-print test?
There was a problem hiding this comment.
ok so the issue was that I was running it from skopeo/vendor for quickly verifying a patch. I've pushed another fix
There was a problem hiding this comment.
ok so the issue was that I was running it from skopeo/vendor for quickly verifying a patch.
How? github.com/projectatomic/skopeo/vendor/github.com/containers/image does not contain a copy of this Makefile at all.
There was a problem hiding this comment.
I copied over this repository so that I could test directly with Skopeo. I agree it is a weird case, and not something you probably want to support, so feel free to close this PR
There was a problem hiding this comment.
Yeah, it is a weird case, and to be clear, not at all supported. But the change is innocuous enough, I guess, so I’ll let @runcom decide.
I was using a repository inside skopeo/vendor and the tests were skipped. Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe
force-pushed
the
fix-make-test
branch
from
3d87088 to
53a117d
Compare
|
👍 @runcom deferring to your decision. |
|
If I may jump in, I don't think this is a good idea; |
could you look at the last rev of this PR? I think it's safe to pull this in now. |
|
yep LGTM |
|
LGTM |
4 participants
Read this bit that got lostwith commit 0138186
Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com