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

migrate test-infra to testify for ddl pkg #27180

Closed
34 tasks done
Tracked by #26022
tisonkun opened this issue Aug 13, 2021 · 23 comments
Closed
34 tasks done
Tracked by #26022

migrate test-infra to testify for ddl pkg #27180

tisonkun opened this issue Aug 13, 2021 · 23 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement.

Comments

@tisonkun
Copy link
Contributor

tisonkun commented Aug 13, 2021

@tisonkun tisonkun added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Aug 13, 2021
@karuppiah7890
Copy link
Contributor

/assign

karuppiah7890 added a commit to karuppiah7890/issues-info that referenced this issue Aug 29, 2021
karuppiah7890 added a commit to karuppiah7890/issues-info that referenced this issue Aug 29, 2021
Signed-off-by: Karuppiah Natarajan <[email protected]>
@tisonkun
Copy link
Contributor Author

tisonkun commented Aug 30, 2021

@karuppiah7890 glad to see you working on this!

ddl is one of our most complicated packages and domains. Even if we just do tests migration, I suggest you start from child package like ddl/label, ddl/placement, etc.

You can collaborate with me as how @yedamao did #27618 (comment) .

@karuppiah7890
Copy link
Contributor

Sure! Thanks @tisonkun ! I did think that this might be a hard one, looking at no one assigned themselves to it yet. I was planning on checking the package today to understand if it needs more subtasks, as I noticed more sub tasks for other issues. Looks like it does. I'll start from small packages that you mentioned 👍

I'll also note down all the test files and sub packages so that we can create sub tasks? Either GitHub issues or put a list in this issue as task list

@tisonkun
Copy link
Contributor Author

I'll also note down all the test files and sub packages so that we can create sub tasks? Either GitHub issues or put a list in this issue as task list

Either #27618 (comment) or #27142 (comment) is ok for me.

@karuppiah7890
Copy link
Contributor

Hoping we won't have too much complex things to comment in the GitHub issues, I'll choose to create just subtasks here for now for easy managment instead of creating too many GitHub issues. But yeah, while creating PRs, we can't close this issue with Fixes #.... Let's see. First let me create a list like this - #27142 (comment)

@karuppiah7890
Copy link
Contributor

Ah, #27142 (comment) also has subtasks as GitHub issues, okay!

@karuppiah7890

This comment has been minimized.

karuppiah7890 added a commit to karuppiah7890/issues-info that referenced this issue Aug 30, 2021
Signed-off-by: Karuppiah Natarajan <[email protected]>
karuppiah7890 added a commit to karuppiah7890/issues-info that referenced this issue Aug 30, 2021
Signed-off-by: Karuppiah Natarajan <[email protected]>
@tisonkun

This comment has been minimized.

@tisonkun
Copy link
Contributor Author

@karuppiah7890 I created issues for all child packages. For files, I'd like to postpone a bit until you'd actually work on it. Maybe follow #27618 (comment) and ask me to update the description, or ask me to create the issues.

@karuppiah7890 karuppiah7890 removed their assignment Sep 2, 2021
@karuppiah7890
Copy link
Contributor

I removed my assignment @tisonkun

@AR10X
Copy link

AR10X commented Oct 8, 2021

I would like to contribute. But I would appreciate it if you help me with how to tackle this issue.

@karuppiah7890
Copy link
Contributor

I think @tisonkun can help find a good first issue among the sub issues of this issue. Some of them are already marked as good first issue, but I'm not sure, I'll have to look at it to understand complexity etc, and I think @tisonkun has more context

@AR10X
Copy link

AR10X commented Oct 8, 2021

Thank you @tisonkun 😄. I will also look at issues see which one I can tackle.

@tisonkun
Copy link
Contributor Author

tisonkun commented Nov 6, 2021

@zimulala @tangenta I just notice that many tests under ddl package are tightly coupled and hardly break down to deal with one by one. Could you please take a look?

@hawkingrei
Copy link
Member

Please add #30086 to the issue description.

@hawkingrei
Copy link
Member

@zimulala @tangenta I just notice that many tests under ddl package are tightly coupled and hardly break down to deal with one by one. Could you please take a look?

OK, I will.

@tisonkun
Copy link
Contributor Author

Please add #30086 to the issue description.

Updated.

@Adam-Kulju
Copy link

Adam-Kulju commented Dec 7, 2021

@tisonkun Migrating schema_test, stat_test, and table_test to testify all spill over into restart_test so those will all have to be done at once I think. I'm working on them right now, hopefully restart_test won't end up spilling over into even more tests (if that's the case I'll probably give up).
Edit: Sadly, touching table_test affects almost every other test file, and I am quite certain migrating all of those at once is beyond my abilities to do. I'll look for some other ones that aren't quite as dependent.

@karuppiah7890 I created issues for all child packages. For files, I'd like to postpone a bit until you'd actually work on it. Maybe follow #27618 (comment) and ask me to update the description, or ask me to create the issues.

@GOODBOY008
Copy link

Hi,@tisonkun . testkit.go and testkit_test.go should migrate test-infra to testify too.

@tisonkun
Copy link
Contributor Author

@GOODBOY008 I don't know what you mean. There is no such file in ddl package. If you mean that in util/testkit folder, please see its successors in testkit folder.

Also, existing migration examples should help.

@GOODBOY008
Copy link

GOODBOY008 commented Dec 17, 2021

@tisonkun I got it.Thanks your patience explanation!

@xhebox
Copy link
Contributor

xhebox commented Jan 20, 2022

#31844 is added to the task list, which is introduced by tiflash guys.

@tisonkun
Copy link
Contributor Author

Finally we finish it! Closed as all subtasks resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

7 participants