-
-
Notifications
You must be signed in to change notification settings - Fork 650
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
Ginkgo should error if a package does not have a suite file #344
Comments
hmm. sorry to sound dense but can you tell me exactly what you were doing and what you saw? Ginkgo doesn't need a |
I made a repo that shows the problem: https://github.com/benmoss/ginkgo-repro If you try to run
Apologies if I was a little terse in my description, I'm gonna see if I can put together a PR later today :) |
welp. that's a totes legit edge case. We could do something ugly like bail if we can't find a line that matches Either way, a PR would be awesome @benmoss |
I don't feel like this is a positive addition to ginkgo, showing a warning is ok, but breaking the build because a package doesn't have a suite test file is too extreme. |
@andrehp we pulled in a change that prints a warning. Are you seeing builds break? |
Yes, I'm seeing breaking builds, I'd guess the problem is at line 467 https://github.com/onsi/ginkgo/blob/master/ginkgo/testrunner/test_runner.go#L467 It sets passed to false when it detects no tests were run. |
@onsi so, any chance of reverting this change? Or at least removing line 467? |
Yes. Fastest way would be a PR that removes that line if you're up for submitting one.
… On Apr 25, 2017, at 5:59 AM, Andre Hahn Pereira ***@***.***> wrote:
@onsi so, any chance of reverting this change? Or at least removing line 467?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Created it |
FWIW this shouldn't be breaking your test suite if you don't have any tests in a package. It should only break in the case where you have Ginkgo tests but no suite file, in which case none of your tests are running anyway, which seems like a thing you'd want to know about. Can you give an example of the problem you're encountering before we merge this? I feel like something else may be the problem here. I've tested this matrix of possibilities: package has go
package has ginkgo test file but no suite_test OR go
Package has no ginkgo or
|
And this change didn't change the behavior of what ginkgo does when there are no test suites, so I feel like this actually makes it more consistent. The wrinkle there is that ginkgo will fail if run against 1 package with no suite, but will pass if run against multiple packages where only some have a suite. It will now fail if they both lack a suite and have a
|
So, the case this change is breaking my build, a project runs ginkgo with:
Where TEST_PACKAGES is generated by
In the case of the project it is
But the testing folder does not actually contain tests, it just contains some mocks and helpers, and ginkgo outputs:
The actual command run is:
I reverted to the last commit before your PR and everything works fine here. The testing package does not contain either suite or _test.go files. |
@andrehp can you tell me more about what files are in your |
@benmoss The project in question is: https://github.com/topfreegames/offers |
It looks like it's due to the |
ok thanks. merging it in. |
Just wasted ~20 minutes of my life figuring out why Ginkgo couldn't find my test file before remembering that a suite file is required. Maybe there could be a warning?
The text was updated successfully, but these errors were encountered: