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

feat(tiflash): Run gtest_10x for unit test when available #3398

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

breezewish
Copy link
Contributor

@breezewish breezewish commented Feb 24, 2025

When there is tests/gtest_10x.py, use it to run unit tests.

This new test runner is introduced in pingcap/tiflash#9899

Copy link

ti-chi-bot bot commented Feb 24, 2025

[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 wuhuizuo for approval. For more information see the Code Review Process.

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

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

Copy link

ti-chi-bot bot commented Feb 24, 2025

I have already done a preliminary review for you, and I hope to help you do a better job.

The pull request title is "feat(tiflash): Run gtest_10x for unit test when available", and it appears to be an enhancement to the existing codebase of a project called "tiflash".

Key Changes:

  1. A new boolean variable use_gtest_10x is introduced which is utilized to check if gtest_10x.py exists.
  2. The fileExists method is used to check if proxy cache and gtest_10x.py file exists.
  3. The script for linking unit test directory is moved into a script block.
  4. Two separate stages for running tests are added: "Run Tests" and "Run Tests (10x)". The stage "Run Tests (10x)" is only executed when gtest_10x.py exists.

Potential Problems:

  1. There is a change in how the script for linking the unit test directory is executed. If there are any dependencies on the previous method of execution, this could cause issues.
  2. The addition of a separate stage for running tests using gtest_10x.py could add complexity and potential failure points to the pipeline.
  3. The changes have been made in two files merged_unit_test.groovy and pull_unit_test.groovy. If there are any other files in the pipeline that depend on these scripts, it could lead to potential compatibility issues.

Fixing Suggestions:

  1. Test the new script execution method thoroughly to ensure it doesn't break any existing functionality.
  2. Have a fallback method in case gtest_10x.py does not exist or fails to execute.
  3. Check all dependent files in the pipeline to ensure compatibility with the changes made in the two files.
  4. More error handling might be needed around the new stages added, especially around cache and file operations.

@ti-chi-bot ti-chi-bot bot added the size/L label Feb 24, 2025
@breezewish
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

1 participant