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

Q: While migrating to V2, my specs were skiped #931

Closed
ycyaoxdu opened this issue Feb 11, 2022 · 6 comments
Closed

Q: While migrating to V2, my specs were skiped #931

ycyaoxdu opened this issue Feb 11, 2022 · 6 comments

Comments

@ycyaoxdu
Copy link

Hi, I'm really appreciate the new feature Spec Decorators, especially the Ordered key word, which may help to split huge It block into some more seprated blocks, make my code more clear.

So i'm trying to migrate to v2.

And while I just changed the import name, from "github.com/onsi/ginkgo" to ginkgo "github.com/onsi/ginkgo/v2", and rerun my test, it turns out to this:

================================================================================
Random Seed: 1644552337

Will run 0 of 0 specs
Running in parallel across 7 processes

Ran 0 of 0 Specs in 0.007 seconds
SUCCESS! -- 0 Passed | 0 Failed | 0 Pending | 0 Skipped


Ginkgo ran 1 suite in 2.174728997s
Test Suite Passed

seems my specs are all skiped.

and i'm followed this recommendation Because of this it is not possible to import and use V1 and V2 in the same package. , i've changed all imports in this package from v1 to v2.

FYI: Change the import path from "github.com/onsi/ginkgo" to ginkgo "github.com/onsi/ginkgo/v2" is all I did to test code. And my test command is $ go test -v ./test/e2e/clusteradm -timeout 1800s

In my cognition, v2 is compatible to v1 styled code.
So I'm confused. Can someone help me with this question? thanks a lot.

@onsi
Copy link
Owner

onsi commented Feb 11, 2022

hey @ycyaoxdu sorry you're having trouble. yes, for the most part, it should be as simple as changing github.com/onsi/ginkgo to github.com/onsi/ginkgo/v2. Is there any chance your repo is open source so I can take a look?

Some other things to consider:

  • make sure there aren't any build tags preventing specs from being compiled in
  • try making a new temporary folder in your folder and bootstrapping and running a new small suite with the ginkgo bootstrap command. this can help confirm there's nothing strange going on with your go setup.

@ycyaoxdu
Copy link
Author

hey @ycyaoxdu sorry you're having trouble. yes, for the most part, it should be as simple as changing github.com/onsi/ginkgo to github.com/onsi/ginkgo/v2. Is there any chance your repo is open source so I can take a look?

Some other things to consider:

  • make sure there aren't any build tags preventing specs from being compiled in
  • try making a new temporary folder in your folder and bootstrapping and running a new small suite with the ginkgo bootstrap command. this can help confirm there's nothing strange going on with your go setup.

yes, i'm working on a open source repo, and my test code is in https://github.com/open-cluster-management-io/clusteradm/pull/111/files, mainly the following dour files:

test/e2e/clusteradm/e2e_suite_test.go
test/e2e/clusteradm/joinhubscenario_bootstrsptoken_test.go
test/e2e/clusteradm/joinhubscenario_sa_test.go
test/e2e/clusteradm/joinhubscenario_timeout_test.go

in directory test/e2e/clusteradm.
and my test command is : go test -v ./test/e2e/clusteradm -timeout 1800s.
and my tests run result: https://github.com/open-cluster-management-io/clusteradm/pull/111/checks#step:4:838

Feeling very sorry to bothering you! i'm not familiar with ginkgo, maybe i just forgot something important.
I'll try to solve this issue by following your suggestions first. Thanks a lot!

@onsi
Copy link
Owner

onsi commented Feb 14, 2022

thanks @ycyaoxdu - on the whole things look ok except for one exception. It's actually an error to run ginkgo.By inside the body of a container node like ginkgo.Context or ginkgo.Describe. By should only be placed inside setup nodes (BeforeEach, AfterEach) and subject nodes (It) as it only applies during the run phase of the suite.

I would have expected you to get a meaningful error in V2 telling you that By should not be placed there but it's possible that error isn't propagating?

Can you try two things for me:

  1. Can you try running the suite with the ginkgo cli instead of go test? I just want to rule out that errors aren't being displayed by go test.

  2. Can you remove all the ginkgo.By statements that aren't in a BeforeEach, AfterEach or It. For example, remove Bys that look like https://github.com/ycyaoxdu/clusteradm/blob/modify-e2e-test/test/e2e/clusteradm/joinhubscenario_timeout_test.go#L14 and https://github.com/ycyaoxdu/clusteradm/blob/modify-e2e-test/test/e2e/clusteradm/joinhubscenario_timeout_test.go#L30

and then run the tests (either with go test or ginkgo).


Lastly I notice you're using done ginkgo.Done in a few places. That has been deprecated. You can use the migration guide to learn how to change your code.

Feeling very sorry to bothering you! i'm not familiar with ginkgo, maybe i just forgot something important.
I'll try to solve this issue by following your suggestions first. Thanks a lot!

No problem at all, am happy to help.

@ycyaoxdu
Copy link
Author

I extend my sincere thanks to you! Your eyes are so sharp! :) @onsi


The issue is actually caused by the misuse of ginkgo.By. Following 2. :

  1. Can you remove all the ginkgo.By statements that aren't in a BeforeEach, AfterEach or It.

my test spec works again.


Lastly I notice you're using done ginkgo.Done in a few places. That has been deprecated.

Hahaha, already migrated! And that is the first thing i did after fixed the bug.
Also, DeferCleanup is really a cool feature!


By the way, I think that the misuse of ginkgo.By should be reported. If the same problem happen to another person, it will leads to an error/warning, pointing out ginkgo.By is misused, while the test starts.
What do you think?

If this proposal will be considered, I'm willing to contribute to this small feature.

@onsi
Copy link
Owner

onsi commented Feb 15, 2022

thanks for the kind words @ycyaoxdu

There should have been an error that appeared but it looks like it wouldn't have been a helpful one. I'm fixing that now and will push out a fix soon.

In addition, it looks like the defer GinkgoRecover() calls that you have in your container nodes (e.g. here) prevented the (unhelpful) error from displaying. The updated version will not have that issue however you should probably remove those calls to defer GinkgoRecover(). In general you should only call defer GInkgoRecover() if you are launching a groutine from within your spec.

@onsi
Copy link
Owner

onsi commented Feb 15, 2022

I just released v2.1.3 which emits an error if By is placed in a container node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants