Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem in version 1.14.0 Multiple Suites Same Package #704

Closed
saarrgett opened this issue Jul 15, 2020 · 9 comments
Closed

Problem in version 1.14.0 Multiple Suites Same Package #704

saarrgett opened this issue Jul 15, 2020 · 9 comments
Labels
v2 Issues that will be resolved by v2 waiting-for-response

Comments

@saarrgett
Copy link

I noticed that after updating to the new release.
tests started to fail in case there are multiple suites under the same package.
This is the error message:
You may only call BeforeEach from within a Describe, Context or When

I can't say for sure what is the cause.
but it started once download to latest release,

@onsi
Copy link
Owner

onsi commented Jul 15, 2020

there was a subtle change introduced to the testing lifecycle in 1.14. Can you share some code so i can reproduce what you're seeing?

@michaelbeaumont
Copy link

michaelbeaumont commented Aug 19, 2020

Hi, I'm experiencing this issue as well, see eksctl-io/eksctl#2561.
At https://github.com/weaveworks/eksctl/blob/0f4f59afd63f6b67ce96eba0cf8683bb30ad8384/pkg/gitops/gitops_test.go#L39, which is in package gitops, and https://github.com/weaveworks/eksctl/blob/0f4f59afd63f6b67ce96eba0cf8683bb30ad8384/pkg/gitops/url_test.go in package gitops_test, I get the error:

profile
/home/mike/projects/weaveworks/eksctl/pkg/gitops/url_test.go:17
  RepositoryURL
  /home/mike/projects/weaveworks/eksctl/pkg/gitops/url_test.go:18
    returns Git URLs as-is [It]
    /home/mike/projects/weaveworks/eksctl/pkg/gitops/url_test.go:19

    You may only call BeforeEach from within a Describe, Context or When

    /home/mike/projects/weaveworks/eksctl/pkg/gitops/gitops_test.go:39

If I remove the url_test.go file, the error disappears.

I've pushed a commit to that PR which fixes the failure.

michaelbeaumont added a commit to michaelbeaumont/eksctl that referenced this issue Aug 19, 2020
@onsi
Copy link
Owner

onsi commented Aug 19, 2020

hmm. i'm seeing url_test.go call testutils.RegisterAndRun(t). You should only call Ginkgo's RunSpecs once in any given package. Calling it twice will lead to duplicate test runs in serial and poor/undefined behavior when running tests in parallel. On Ginkgo's end I should make it clearer that it is an error to call RunSpecs more than once (both in the documentation and at run-time). 1.14 made a few changes to the test lifecycle but I don't think it introduced an issue (though I could be wrong!) but rather has made calling RunSpecs twice fail more readily.

All this to say that removing these lines should fix the eksctl tests.

I share a bit more detail in this issue: #708

@michaelbeaumont
Copy link

Might be my golang ignorance showing here but if url_test.go is the only file in package gitops_test then we have to call RunSpecs there, right?

@onsi
Copy link
Owner

onsi commented Aug 19, 2020

ah, i see. i'm a bit rusty on the distinction (and am not near a dev environment) but I leave the idea is that the _test package is compiled as a separate package under the covers and then linked in. This is to help surface package export/naming issues. Because it's linked in the resulting test binary basically has everything in the _test and the non-_test packages.

As far as Ginkgo is concerned any Ginkgo nodes (i.e. It, Describe, etc.) defined in either the _test or non-_test package will be combined and run by the single RunSpecs invocation. Since you have a gitops_suite_test.go file with that invocation that should be all you need even in mixed-package mode where some files are in _test and some are not.

If you see any behavior that contradicts that please let me know and I'll put together a reproducer locally once I get to my dev machine.

@michaelbeaumont
Copy link

Ah got it, so once per package directory thingy. That change seems to work, thanks!

michaelbeaumont added a commit to michaelbeaumont/eksctl that referenced this issue Aug 19, 2020
@jmalloc
Copy link

jmalloc commented Dec 24, 2020

I have hit this problem too, specifically when using go test -count=n to run tests, where n > 1.

Using the test below under Ginkgo v1.14+ I see the failure as described by @saarrgett. It works as expected under Ginkgo v1.13.0. Edit: I should say it always appears to run as expected under v1.13 ;)

package ginkgotest_test

import (
	"testing"

	"github.com/onsi/ginkgo"
)

var _ = ginkgo.Describe("<describe>", func() {
	ginkgo.BeforeEach(func() {
	})

	ginkgo.It("passes", func() {
	})
})

func TestSuite(t *testing.T) {
	ginkgo.RunSpecs(t, "<suite>")
}

Console output with v1.13.0:

❯ go get github.com/onsi/ginkgo@v1.13.0
❯ go test -count=2 -v .
=== RUN   TestSuite
Running Suite: <suite>
======================
Random Seed: 1608768228
Will run 1 of 1 specs

•
Ran 1 of 1 Specs in 0.000 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestSuite (0.00s)
=== RUN   TestSuite
Running Suite: <suite>
======================
Random Seed: 1608768228
Will run 1 of 1 specs

•
Ran 1 of 1 Specs in 0.000 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestSuite (0.00s)
PASS
ok  	ginkgotest	0.135s

Console output with v1.14.0:

❯ go get github.com/onsi/ginkgo@v1.14.0
❯ go test -count=2 -v .
=== RUN   TestSuite
Running Suite: <suite>
======================
Random Seed: 1608768279
Will run 1 of 1 specs

•
Ran 1 of 1 Specs in 0.000 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestSuite (0.00s)
=== RUN   TestSuite
Running Suite: <suite>
======================
Random Seed: 1608768279
Will run 2 of 2 specs

• Failure in Spec Setup (BeforeEach) [0.000 seconds]
<describe>
/tmp/ginkgotest/pkg_test.go:9
  passes [BeforeEach]
  /tmp/ginkgotest/pkg_test.go:13

  You may only call BeforeEach from within a Describe, Context or When

  /tmp/ginkgotest/pkg_test.go:10
------------------------------
•

Summarizing 1 Failure:

[Fail] <describe> [BeforeEach] passes 
/tmp/ginkgotest/pkg_test.go:10

Ran 2 of 2 Specs in 0.000 seconds
FAIL! -- 1 Passed | 1 Failed | 0 Pending | 0 Skipped
--- FAIL: TestSuite (0.00s)
FAIL
FAIL	ginkgotest	0.288s
FAIL

@onsi
Copy link
Owner

onsi commented Jan 11, 2021

yeah -count isn't supported by Ginkgo. I commented on this here:
#485 (comment)

I'm planning on (eventually) updating Ginkgo to abort and explain when users use -count.

Out of curiosity I'd love to understand if there are circumstances that require you to use go test instead of the ginkgo cli.

@jmalloc
Copy link

jmalloc commented Jan 11, 2021

yeah -count isn't supported by Ginkgo. I commented on this here:

Ok, no problem. I could have sworn this did actually work, but even if it did and it was by chance I understand it's not a supported feature.

Out of curiosity I'd love to understand if there are circumstances that require you to use go test instead of the ginkgo cli.

There is no technical reason I can point to. That is, it's not because the ginkgo CLI is lacking in any way that we're aware of. Rather, our use of go test arose naturally as part of my workplace's general effort to standardise tooling across our projects.

Some of our projects use Ginkgo, other's don't, some projects use "testable examples" or even just "regular Go tests" alongside Ginkgo test, etc, etc. Being able to run go test without having to understand how the tests are written has been a positive for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2 Issues that will be resolved by v2 waiting-for-response
Projects
None yet
Development

No branches or pull requests

5 participants