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

[Feature] Run Ginkgo tests with Ginkgo CLI #2658

Open
1 of 2 tasks
MortalHappiness opened this issue Dec 17, 2024 · 11 comments
Open
1 of 2 tasks

[Feature] Run Ginkgo tests with Ginkgo CLI #2658

MortalHappiness opened this issue Dec 17, 2024 · 11 comments
Assignees
Labels
ci enhancement New feature or request good first issue Good for newcomers

Comments

@MortalHappiness
Copy link
Member

MortalHappiness commented Dec 17, 2024

Search before asking

  • I had searched in the issues and found no similar feature requirement.

Description

Currently, Ginkgo tests are run with go test, and we cannot run a single test, which is annoying. We should use the Ginkgo CLI to run Ginkgo tests.

Implementation Details

  • Please search for Describe( using your IDE to identify which files contain Ginkgo tests.
  • Try running the tests locally using the Ginkgo CLI and fix any errors that occur.
  • Attempt to run a single test with the Ginkgo CLI; it should pass. Update the test to intentionally make it fail, then run it again with the Ginkgo CLI to ensure it fails as expected.
  • Update the configurations in the .github/ and .buildkite/ folders to use the Ginkgo CLI instead of go test for running Ginkgo tests.

Use case

No response

Related issues

No response

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!
@MortalHappiness MortalHappiness added enhancement New feature or request good first issue Good for newcomers ci labels Dec 17, 2024
@nadongjun
Copy link
Contributor

@MortalHappiness Hi! I’d like to take this on—please assign it to me if possible. Thanks!

@dentiny
Copy link
Contributor

dentiny commented Dec 17, 2024

@MortalHappiness
Copy link
Member Author

Does go test -run work here? Eg: https://stackoverflow.com/questions/26092155/just-run-single-test-instead-of-the-whole-suite

No, it only works for regular Golang tests, not Ginkgo tests.

@dentiny
Copy link
Contributor

dentiny commented Dec 17, 2024

No, it only works for regular Golang tests, not Ginkgo tests.

Hmm would this work?
https://stackoverflow.com/questions/68008485/how-to-run-a-single-spec-using-ginkgo-test-framework

@nadongjun
Copy link
Contributor

nadongjun commented Dec 17, 2024

I've tested running specific Ginkgo tests using go test -v -ginkgo.focus "Describe", -ginkgo.skip "Describe", and it works as expected.

Given these benefits, do you think switching from go test to the Ginkgo CLI would be a good move?

@MortalHappiness
Copy link
Member Author

do you think switching from go test to the Ginkgo CLI would be a good move?

Yes. I still think it's better to switch to use Ginkgo CLI, because it has more features.

@andrewsykim
Copy link
Collaborator

I forgot about -ginkgo.focus, for a short-term fix can we allow setting ginkgo.focus with make test? That should be a short/simple PR that we can do while implementing make test to use Ginkgo CLI

@nadongjun
Copy link
Contributor

Upon reviewing the issue, it was confirmed that a conflict occurs when Ginkgo tests are mixed with standard Go tests. (When running Ginkgo --focus, all tests defined in the standard go test are also executed.) This happens because, to use Ginkgo with go test, all test code must be written in the Ginkgo format. The flags -ginkgo.focus and -ginkgo.skip only work with tests written in the Ginkgo framework.

Conflicts can arise when Ginkgo tests are mixed with standard Go tests, and to resolve this, all tests need to be standardized to the Ginkgo format. Converting all tests to the Ginkgo format will require considerable time and effort, making it a long-term task. For further reference on similar issues, you can check Ginkgo Issue #358.

@MortalHappiness
Copy link
Member Author

Could we use ginkgo convert as mentioned in that issue?

@andrewsykim
Copy link
Collaborator

As a temporary measure, can we introduce a new make target that just runs the ginkgo tests, with support for passing in a focus string? We dont need to run it through CI, it's just for local development

@nadongjun
Copy link
Contributor

nadongjun commented Dec 23, 2024

@MortalHappiness
Regarding the use of ginkgo convert, it has been deprecated starting from Ginkgo v2.

@andrewsykim
Sure, I've added a new make target that runs the Ginkgo tests with the option to pass in a focus string. This is intended for local development, so there's no need to run it through CI. The target requires the TEST_NAME and the path to the test folder to be specified. If go test is available, it runs the test with the provided TEST_NAME. If not, it falls back to running the Ginkgo tests with the focus string. Here's how it works:

Script:

test-target: WHAT ?= ./controllers/ray/
test-target: TEST_NAME ?= ""
test-target: ENVTEST_K8S_VERSION ?= 1.24.2
test-target: manifests fmt vet envtest
	@go test -run $(TEST_NAME) $(WHAT) -count=1 | tee >(if grep -q "no tests to run"; then \
		echo "No matching Go tests found. Running Ginkgo tests…"; \
		KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) —bin-dir $(LOCALBIN) -p path)" ginkgo —focus "$(TEST_NAME)" $(WHAT); \
	fi) 

For ginkgo test:

make test-target TEST_NAME=RayClusterwithinvalidNumOfHosts WHAT=./controllers/ray/

For go test:

make test-target TEST_NAME=TestCreateRayJobSubmitterIfNeed

If this approach looks reasonable, I will proceed to create a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants