test(nodeaffinitywithcache): migrate Ginkgo/Gomega tests to standard testing with testify by hxrshxz · Pull Request #5756 · fluid-cloudnative/fluid

hxrshxz · 2026-04-01T17:17:15Z

Ⅰ. Describe what this PR does

Migrate pkg/webhook/plugins/nodeaffinitywithcache/ tests from Ginkgo/Gomega to standard Go testing with testify/assert and testify/require. Removes the Ginkgo suite bootstrap and vendors testify/require.

Ⅱ. Does this pull request fix one issue?

fixes #5676

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

node_affinity_with_cache_test.go: converted 5 Ginkgo Describe/It groups (9 specs) to TestNewPluginAndGetName, TestGetPreferredSchedulingTerm, TestMutateOnlyRequired, TestMutateOnlyPrefer, TestMutateBothRequiredAndPrefer, and 4 TestTieredLocality_* functions; extracted repeated YAML locality strings to package-level constants.
tiered_locaity_test.go: converted DescribeTable/Entry (4 entries) to TestHasRepeatedLocality with 4 table-driven sub-tests.
nodeaffinitywithcache_suite_test.go: deleted (Ginkgo runner bootstrap no longer needed).
vendor/github.com/stretchr/testify/require/: vendored testify/require (already declared in go.mod; only assert was previously vendored).

Ⅳ. Describe how to verify it

Run go test ./pkg/webhook/plugins/nodeaffinitywithcache/... -count=1 and confirm all tests pass with no Ginkgo or Gomega imports in the test files. Coverage: 87.2%.

Ⅴ. Special notes for reviews

N/A

hxrshxz · 2026-04-01T17:18:14Z

/gemini review

gemini-code-assist

Code Review

This pull request migrates the test suite from Ginkgo/Gomega to the standard library's testing package and testify/assert/testify/require libraries. The reviewer suggests optimizing the newTestScheme function by initializing the scheme once as a package-level variable to reduce redundant allocations and recommends removing the assertion on the input map length in TestMutateBothRequiredAndPrefer as it does not test the plugin's mutation logic.

gemini-code-assist · 2026-04-01T17:23:00Z

pkg/webhook/plugins/nodeaffinitywithcache/node_affinity_with_cache_test.go

+func newTestScheme(t *testing.T) *runtime.Scheme {
+	t.Helper()
+	schema := runtime.NewScheme()
+	require.NoError(t, datav1alpha1.AddToScheme(schema))
+	require.NoError(t, corev1.AddToScheme(schema))
+	return schema
+}


The newTestScheme function is called in every test case. Since runtime.Scheme is thread-safe for reading after initialization and the scheme used here is static, you could initialize it once as a package-level variable to avoid redundant allocations and setup in every test.

gemini-code-assist · 2026-04-01T17:23:01Z

pkg/webhook/plugins/nodeaffinitywithcache/node_affinity_with_cache_test.go

+	require.NotNil(t, schedPod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution)
+	assert.Len(t, schedPod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms, 1)
+	assert.Len(t, schedPod.Spec.Affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution, 1)
+	assert.Len(t, runtimeInfos, 2)


This assertion checks the length of the input map runtimeInfos. Since Mutate does not modify the input map, this assertion doesn't provide value for testing the plugin's logic. It can be removed to keep the test focused on the mutation results.

Copilot

Pull request overview

Migrates nodeaffinitywithcache plugin tests from Ginkgo/Gomega to Go’s standard testing package using testify/assert and testify/require, and removes the Ginkgo suite bootstrap.

Changes:

Converted Ginkgo specs/tables to testing.T tests and table-driven subtests with testify.
Removed the Ginkgo suite runner (*_suite_test.go).
Updated vendoring metadata to include github.com/stretchr/testify/require.

Reviewed changes

Copilot reviewed 3 out of 11 changed files in this pull request and generated 4 comments.

File	Description
vendor/modules.txt	Adds `testify/require` to vendored module package list.
pkg/webhook/plugins/nodeaffinitywithcache/tiered_locaity_test.go	Replaces Ginkgo table tests with `testing.T` + `testify` table-driven subtests.
pkg/webhook/plugins/nodeaffinitywithcache/nodeaffinitywithcache_suite_test.go	Deletes the Ginkgo bootstrap runner.
pkg/webhook/plugins/nodeaffinitywithcache/node_affinity_with_cache_test.go	Rewrites multiple Ginkgo suites into standard Go tests and adds shared test helpers/constants.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/webhook/plugins/nodeaffinitywithcache/tiered_locaity_test.go

pkg/webhook/plugins/nodeaffinitywithcache/node_affinity_with_cache_test.go

gemini-code-assist

Code Review

This pull request migrates the nodeaffinitywithcache plugin tests from Ginkgo/Gomega to the standard Go testing package using testify/assert and testify/require. The refactoring includes converting existing test suites into standard Go test functions and table-driven tests, alongside updating the vendor directory. Feedback suggests further enhancing the readability and isolation of the new tests by utilizing t.Run subtests instead of manual state resets.

pkg/webhook/plugins/nodeaffinitywithcache/node_affinity_with_cache_test.go

…level var Replace per-test newTestScheme(t) function with a package-level testScheme var initialized once via an immediately-invoked function. This avoids redundant scheme registration on every test invocation. Addresses Gemini review feedback on PR fluid-cloudnative#5756. Signed-off-by: Harsh <harshmastic@gmail.com>

hxrshxz · 2026-04-01T17:38:24Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors the test suite for the nodeaffinitywithcache plugin, migrating from Ginkgo/Gomega to the standard Go testing package and testify. The review feedback suggests several improvements to the new tests, including handling errors during scheme initialization, refactoring multi-scenario tests into subtests using t.Run for better isolation, and adding missing assertions to verify that no mutations occur in specific edge cases.

pkg/webhook/plugins/nodeaffinitywithcache/node_affinity_with_cache_test.go

Copilot

Pull request overview

Copilot reviewed 3 out of 11 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

pkg/webhook/plugins/nodeaffinitywithcache/node_affinity_with_cache_test.go

…testing with testify Signed-off-by: Harsh <harshmastic@gmail.com>

…level var Replace per-test newTestScheme(t) function with a package-level testScheme var initialized once via an immediately-invoked function. This avoids redundant scheme registration on every test invocation. Addresses Gemini review feedback on PR fluid-cloudnative#5756. Signed-off-by: Harsh <harshmastic@gmail.com>

codecov · 2026-04-01T18:22:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.40%. Comparing base (2f8fc96) to head (9ec9bb0).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5756   +/-   ##
=======================================
  Coverage   61.40%   61.40%           
=======================================
  Files         444      444           
  Lines       30656    30656           
=======================================
  Hits        18824    18824           
  Misses      10285    10285           
  Partials     1547     1547

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…cate literals and fix function naming Signed-off-by: Harsh <harshmastic@gmail.com>

sonarqubecloud · 2026-04-01T19:20:22Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

fluid-e2e-bot · 2026-04-03T07:06:54Z

Hi @hxrshxz. Thanks for your PR.

I'm waiting for a fluid-cloudnative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

cheyang · 2026-04-04T12:49:13Z

/ok-to-test

cheyang · 2026-04-04T12:49:15Z

Non-blocking comment: this migration looks solid overall. A small follow-up improvement would be to strengthen a few of the converted tests so they assert the resulting affinity structure more directly, not just the no-error path. I do not see this as a blocker for the current PR.

cheyang · 2026-04-04T12:49:17Z

/lgtm

cheyang

/approve

fluid-e2e-bot · 2026-04-04T12:49:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheyang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [cheyang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot AI review requested due to automatic review settings April 1, 2026 17:17

gemini-code-assist bot reviewed Apr 1, 2026

View reviewed changes

Copilot AI reviewed Apr 1, 2026

View reviewed changes

gemini-code-assist bot reviewed Apr 1, 2026

View reviewed changes

pkg/webhook/plugins/nodeaffinitywithcache/node_affinity_with_cache_test.go Show resolved Hide resolved

hxrshxz requested a review from Copilot April 1, 2026 17:38

Copilot started reviewing on behalf of hxrshxz April 1, 2026 17:40 View session

hxrshxz mentioned this pull request Apr 1, 2026

Unify and Modernize Fluid’s Unit Testing Framework and Enhance Testing Coverage #5676

Closed

gemini-code-assist bot reviewed Apr 1, 2026

View reviewed changes

Copilot AI reviewed Apr 1, 2026

View reviewed changes

hxrshxz added 2 commits April 1, 2026 23:18

test(nodeaffinitywithcache): migrate Ginkgo/Gomega tests to standard …

203a9f1

…testing with testify Signed-off-by: Harsh <harshmastic@gmail.com>

hxrshxz force-pushed the migrate-nodeaffinitywithcache-tests branch from 7003265 to 19df57d Compare April 1, 2026 17:49

Copilot started reviewing on behalf of hxrshxz April 1, 2026 18:17 View session

fix(nodeaffinitywithcache): address SonarCloud issues - extract dupli…

9ec9bb0

…cate literals and fix function naming Signed-off-by: Harsh <harshmastic@gmail.com>

fluid-e2e-bot bot added the needs-ok-to-test label Apr 3, 2026

cheyang approved these changes Apr 4, 2026

View reviewed changes

fluid-e2e-bot bot assigned cheyang Apr 4, 2026

fluid-e2e-bot bot added lgtm ok-to-test and removed needs-ok-to-test labels Apr 4, 2026

fluid-e2e-bot bot added the approved label Apr 4, 2026

fluid-e2e-bot bot merged commit 9fe6aad into fluid-cloudnative:master Apr 4, 2026
18 checks passed

Conversation

hxrshxz commented Apr 1, 2026

Ⅰ. Describe what this PR does

Ⅱ. Does this pull request fix one issue?

Ⅲ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Uh oh!

hxrshxz commented Apr 1, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

hxrshxz commented Apr 1, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sonarqubecloud bot commented Apr 1, 2026

Quality Gate passed

Uh oh!

fluid-e2e-bot bot commented Apr 3, 2026

Uh oh!

cheyang commented Apr 4, 2026

Uh oh!

cheyang commented Apr 4, 2026

Uh oh!

cheyang commented Apr 4, 2026

Uh oh!

cheyang left a comment

Choose a reason for hiding this comment

Uh oh!

fluid-e2e-bot bot commented Apr 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

codecov bot commented Apr 1, 2026 •

edited

Loading