Automatically bootstrap ginkgo as a part of generate target #359
Conversation
|
Can one of the admins verify this patch? |
mpolednik
force-pushed
the
ginkgo-bootstrap
branch
7 times, most recently
from
504bfea
to
0e81c21
Compare
Makefile
Outdated
| @@ -8,6 +8,7 @@ generate: | |||
| find pkg/ -name "*generated*.go" -exec rm {} -f \; | |||
| ./hack/build-go.sh generate ${WHAT} | |||
| goimports -w -local kubevirt.io cmd/ pkg/ tests/ | |||
| ./hack/bootstrap-ginkgo.sh test ${WHAT} | |||
There was a problem hiding this comment.
Make generate is being used for formatting and documentation. I don't personally like the idea of running the test suite at that point. We already have the make test and make functest targets. Isn't that enough?
EDIT: Or am I misreading this line because of the test argument, which actually appears to be unused in hack/bootstrap-ginkgo.sh?
There was a problem hiding this comment.
You've spotted an evil copy-paste. :)
It's only supposed to generate the structure, not run the tests. Will fix the tests.
pkg/healthz/healthz_suite_test.go
Outdated
| "testing" | ||
| ) | ||
|
|
||
| func TestHealthz(t *testing.T) { |
There was a problem hiding this comment.
It's not clear to me why these explicit test wrappers are needed. What do we gain?
There was a problem hiding this comment.
They're the minimal test files that ginkgo initializes. We don't gain a lot initially, but we won't forget to initialize ginkgo tests in the future.
|
The objective is not really improving the current suite, but making sure if/when new packages are added or current packages are split, we don't forget to bootstrap ginkgo within them (otherwise, they'll be left out of testing). Ran into the issue myself in previous PR, would prefer to have it done automatically. |
|
@rmohr this is probably something you should weigh in on. |
mpolednik
force-pushed
the
ginkgo-bootstrap
branch
from
0e81c21
to
d712048
Compare
|
Pushed the arg removal, the decision is up to you all. Also note that the generation isn't perfect - it would be nice to have some kind of go package introspection to really only call it in directories where it's needed. |
|
While I would prefer, that the suits are only added, if there is a tests file which needs it, or there would be some kind of check which makes travis fail, I like that it eliminates the mentioned errors. I also found some disabled tests a few days ago, because accidentally such a suit file was removed. |
|
ok to test |
|
OK to test |
|
testing this again. |
|
@mpolednik could you rebase? Sorry for letting that hang so long in approved state. |
hack/bootstrap-ginkgo.sh
Outdated
| if [ "x${GOFILES}" != "x0" ]; then | ||
| # Since ginkgo bootstrap doesn't take path, we have to hop in the | ||
| # target package first. | ||
| (cd $dir && ginkgo bootstrap || :) |
There was a problem hiding this comment.
What came to my mind: How about checking here, if a "*_test.go" file exists in the directory, and only then do the bootstrap?
There was a problem hiding this comment.
Yeah, that could work. Gonna look into it along the rebase ASAP.
mpolednik
force-pushed
the
ginkgo-bootstrap
branch
2 times, most recently
from
e1e26fe
to
01e14a1
Compare
Ginkgo tests require each tested package to be bootstrapped to run. This patch makes the bootstrapping available in makefile's generate target (which is safe automatically in commit hook). The nature of this approach causes multiple new files - these indicate packages that were not really tested by ginkgo. On the other hand, when adding tests for those packages, we are not required to bootstrap them anymore. Signed-off-by: Martin Polednik <mpolednik@redhat.com>
mpolednik
force-pushed
the
ginkgo-bootstrap
branch
from
01e14a1
to
9a532de
Compare
|
retest this please |
libcni, pkg\invoke: Use OS-specific list separator when parsing CNI_PATH
Ginkgo tests require each tested package to be bootstrapped to run.
This patch makes the bootstrapping available in makefile's generate
target (which is safe automatically in commit hook).
The nature of this approach causes multiple new files - these indicate
packages that were not really tested by ginkgo. On the other hand,
when adding tests for those packages, we are not required to bootstrap
them anymore.
Signed-off-by: Martin Polednik mpolednik@redhat.com