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

Tidy up test cases for clustered index #1599

Merged
merged 2 commits into from
Mar 20, 2021

Conversation

JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented Mar 20, 2021

What problem does this PR solve?

Tidy up test cases after #1591

What is changed and how it works?

Proposal: xxx

What's Changed:

How it Works:

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression
    • Consumes more CPU
    • Consumes more MEM
  • Breaking backward compatibility

Release note

@JaySon-Huang JaySon-Huang added the type/testing Issue or PR for testing label Mar 20, 2021
@JaySon-Huang JaySon-Huang requested a review from windtalker March 20, 2021 02:55
@JaySon-Huang JaySon-Huang self-assigned this Mar 20, 2021
@JaySon-Huang JaySon-Huang force-pushed the fix_clustered_index_tests branch from 3b9f158 to b157530 Compare March 20, 2021 03:06
@windtalker
Copy link
Contributor

Hi, I think the ci error for clustered index has been fixed by #1591

@JaySon-Huang JaySon-Huang force-pushed the fix_clustered_index_tests branch from b157530 to b1bf8c0 Compare March 20, 2021 04:32
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Mar 20, 2021

@windtalker Yes, I found that PR after filing this PR because I met an error when merging PRs on release-5.0.
Now I make this PR to tidy up the directory and files under tests. I think We don't need a separate directory for clustered index cause we can run the test without changing the configuration file of TiDB.

@JaySon-Huang JaySon-Huang changed the title [WIP] Fix test cases for clustered index Tidy up test cases for clustered index Mar 20, 2021
source ../../_vars.sh
setup_dylib_path
#source ../../_vars.sh
#setup_dylib_path
Copy link
Contributor

Choose a reason for hiding this comment

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

why need this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a script located in tiflash repo rather than tics repo. This will make an error if we only checkout the tics repo and run the tests.
And I find that I can run the tests without these two lines.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK.

Copy link
Contributor

@windtalker windtalker left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 20, 2021
@JaySon-Huang
Copy link
Contributor Author

/run-all-tests

@JaySon-Huang
Copy link
Contributor Author

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 20, 2021
@JaySon-Huang JaySon-Huang added the needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 label Mar 20, 2021
@ti-srebot
Copy link
Collaborator

/run-all-tests

@ti-srebot ti-srebot merged commit f20dfc6 into pingcap:master Mar 20, 2021
@ti-srebot
Copy link
Collaborator

cherry pick to release-5.0 in PR #1603

@JaySon-Huang JaySon-Huang deleted the fix_clustered_index_tests branch March 21, 2021 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/testing Issue or PR for testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants