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

CI: optimize unit test #492

Merged
merged 6 commits into from
Mar 5, 2025
Merged

Conversation

Song-Quan
Copy link
Contributor

  • Trigger unit tests based on the changed code
  • Extract duplicate code as commands

Copy link

github-actions bot commented Mar 4, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Song-Quan
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

pkg/.* run_unittest true
.bazelrc run_unittest true
.circleci/unittest-config.yml run_unittest true
api/.* run_go_ut true

Choose a reason for hiding this comment

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

The change from run_unittest to run_go_ut seems reasonable given the PR title 'CI: optimize unit test'. However, could you elaborate on why the other mappings (bazel, cmd, engine, pkg, .bazelrc, .circleci/unittest-config.yml) were removed? Are these tests now covered under the run_go_ut or are they being handled differently? Understanding the reasoning behind these removals will help ensure that the change doesn't inadvertently reduce test coverage.

@rivertalk
Copy link

@gemini-code-assist PTAL

Copy link

PTAL means 'Please Take A Look'. I will review the pull request now.

@FollyCoolly
Copy link
Collaborator

@gemini-code-assist explain this PR

Copy link

The PR aims to optimize the unit test process by triggering tests based on changed code and extracting duplicate code into reusable commands. I'll provide a summary of the changes below:

  • .circleci/config.yml: Modified to differentiate between Go and C++ unit tests, triggering different workflows based on file changes.
  • .circleci/unittest-config.yml: Substantial changes including:
    • Introduction of run_go_ut and run_cpp_ut parameters.
    • Definition of new commands: setup_cpp_test, run_go_test, and run_cpp_test to encapsulate test execution logic.
    • Refactoring of existing jobs (linux_scdb_ut, linux_engine_ut, macOS_scdb_ut, macOS_engine_ut) to use the new commands.
    • Addition of new jobs (linux_cpp_ut, macOS_cpp_ut) for C++ unit tests.
  • Workflows: Updated to use run_go_ut and run_cpp_ut workflows, triggering the corresponding jobs based on the defined parameters.

Copy link
Collaborator

@tongke6 tongke6 left a comment

Choose a reason for hiding this comment

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

LGTM

@tongke6
Copy link
Collaborator

tongke6 commented Mar 4, 2025

@jingshi-ant please take a final look

Copy link
Collaborator

@jingshi-ant jingshi-ant left a comment

Choose a reason for hiding this comment

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

LGTM

@jingshi-ant jingshi-ant merged commit 364ac40 into secretflow:main Mar 5, 2025
11 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants