Skip to content

chore(spark): align test patterns and fix cross-module import - #677

Open
KartikSuryavanshi wants to merge 1 commit into
kubeflow:mainfrom
KartikSuryavanshi:test/spark-align-test-patterns
Open

chore(spark): align test patterns and fix cross-module import#677
KartikSuryavanshi wants to merge 1 commit into
kubeflow:mainfrom
KartikSuryavanshi:test/spark-align-test-patterns

Conversation

@KartikSuryavanshi

Copy link
Copy Markdown

What this PR does / why we need it:

This PR addresses test pattern alignment and a cross-module import bug as part of #609.

Changes:

  1. Fix cross-module import in types_test.py: Import from kubeflow.spark.test.common instead of kubeflow.trainer.test.common to remove unnecessary coupling with the Trainer package.

  2. Migrate try/except to pytest.raises() in backend_test.py: Replaced 16 try/except blocks with pytest.raises() context managers for cleaner error assertion patterns, matching the Trainer SDK test style.

  3. Replace string-based branching in utils_test.py: Converted string-based if/elif on test_case.name to data-driven expected_output assertions in:

    • test_build_spark_connect_cr (14 branches to dictionary-based assertions)
    • test_get_spark_connect_info_from_cr (5 branches to generic field checks)
    • test_resolve_driver_resources (3 branches to config-based checks)
    • test_resolve_executor_resources (4 branches to config-based checks)
    • test_generate_session_name, test_generate_job_name, test_build_service_url, test_read_pod_logs
  4. Remove redundant assertions: Removed assert test_case.expected_status == SUCCESS where pytest.raises() already handles the error path.

Validation:

  • All 263 Spark unit tests pass
  • ruff check and ruff format pass cleanly

Part of #609

Copilot AI review requested due to automatic review settings July 28, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@google-oss-prow

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kramaranya for approval. For more information see the Kubernetes Code Review Process.

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

Details Needs approval from an approver in each of these files:

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

@github-actions

Copy link
Copy Markdown
Contributor

🎉 Welcome to the Kubeflow SDK! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

  • If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards
  • Our team will review your PR soon! cc @kubeflow/kubeflow-sdk-team

Join the community:

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

@KartikSuryavanshi
KartikSuryavanshi force-pushed the test/spark-align-test-patterns branch from 2e8d340 to e897a81 Compare July 28, 2026 10:06
- Fix cross-module import in types_test.py: import from kubeflow.spark.test.common
  instead of kubeflow.trainer.test.common
- Migrate 16 try/except blocks to pytest.raises() in backend_test.py for cleaner
  error assertion patterns
- Replace string-based branching with data-driven expected_output assertions in
  utils_test.py (test_build_spark_connect_cr, test_get_spark_connect_info_from_cr,
  test_resolve_driver_resources, test_resolve_executor_resources, etc.)
- Remove redundant assert test_case.expected_status == SUCCESS checks where
  pytest.raises() already handles the error path

Part of kubeflow#609

Signed-off-by: Kartik Suryavanshi <158498247+KartikSuryavanshi@users.noreply.github.com>
@KartikSuryavanshi
KartikSuryavanshi force-pushed the test/spark-align-test-patterns branch from e897a81 to fb257ae Compare July 28, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants