Skip to content

Conversation

Tomlord1122
Copy link
Contributor

@Tomlord1122 Tomlord1122 commented Sep 3, 2025

Why are these changes needed?

This PR refactors test support files to eliminate code duplication and improve maintainability by consolidating common utility functions.

Files Modified:
• ‎ray-operator/test/e2eautoscaler/support.go
• ‎ray-operator/test/e2erayservice/support.go
• ‎ray-operator/test/e2eautoscaler/raycluster_autoscaler_test.go
• ‎ray-operator/test/e2eautoscaler/raycluster_autoscaler_part2_test.go
• ‎ray-operator/test/e2erayservice/rayservice_ha_test.go

Main Improvements:

  1. Consolidated generic utility functions (‎option[T], ‎apply[T], ‎options[T]) to use existing implementations in ‎test/support/support.go
  2. Eliminated duplicate ConfigMap helper functions (‎configMapWith, ‎file, ‎files, ‎mountConfigMap)
  3. Updated function calls to use capitalized versions from the support package
  4. Preserved package-specific behavior (ConfigMap naming) while removing duplication

Solution Details:

Before:

// Duplicated in both e2eautoscaler and e2erayservice
type option[T any] func(t *T) *T
func apply[T any](t *T, options ...option[T]) *T { /* ... */ }
func files(t Test, fileNames ...string) option[...] { /* ... */ }

After:

// Uses existing functions from test/support package
Files(test, "file1.py", "file2.py")
Apply(config, MountConfigMap[Type](cm, "/path"))

Related issue number

Closes #3947

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

…e2erayservice/support.go by reusing test/support/support.go implementations to improve maintainability and reduce redundancy. Related to ray-project#3932

Signed-off-by: HSIU-CHI LIU (Tomlord) <[email protected]>
@Tomlord1122
Copy link
Contributor Author

@Future-Outlier PTAL

Copy link
Contributor

@EagleLo EagleLo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Suggestion: Consider a more concise PR title like "Refactor: Consolidate duplicate test utility functions" for clarity. The CI failure appears to be a flaky test unrelated to your changes.

@Tomlord1122 Tomlord1122 changed the title [Refactor] Remove duplicate function in e2eautoscaler/support.go and … [Refactor] Consolidate Duplicate Test Utilities for Maintainability Sep 3, 2025
@Tomlord1122 Tomlord1122 changed the title [Refactor] Consolidate Duplicate Test Utilities for Maintainability [Refactor] Consolidate duplicate test utilities for maintainability Sep 3, 2025
@Future-Outlier Future-Outlier self-assigned this Sep 12, 2025
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

Successfully merging this pull request may close these issues.

Remove duplicated functions in e2eautoscaler/support.go and e2erayservice/support.go to reuse test/support/support.go
4 participants