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 executor_test.go #28577

Closed
19 tasks done
Tracked by #26854
tisonkun opened this issue Oct 8, 2021 · 11 comments · Fixed by #33709 or #34062
Closed
19 tasks done
Tracked by #26854

migrate test-infra to testify for executor_test.go #28577

tisonkun opened this issue Oct 8, 2021 · 11 comments · Fixed by #33709 or #34062
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. 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 Oct 8, 2021

All tests listed below is moved to executor_legacy_test.go file.

@karuppiah7890
Copy link
Contributor

/assign

@tisonkun tisonkun added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Oct 8, 2021
karuppiah7890 added a commit to karuppiah7890/issues-info that referenced this issue Oct 8, 2021
Signed-off-by: Karuppiah Natarajan <[email protected]>
@karuppiah7890
Copy link
Contributor

I see lot of test suite setup and tear down as part of this test. Just from an overview, it looks like we can also do the setup at test level. But if we still want to do the setup at test suite level - we can choose to use TestMain but that would be doing setup for all tests in the package and not just executor_test.go test, or we can try to use testify's test suite feature. Let me know what you folks think

cc @tisonkun

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

Hi @karuppiah7890 ! There is a similar case #28300 you can refer to. I think it can directly answer your question. Let me know if still something unclear.

@karuppiah7890
Copy link
Contributor

karuppiah7890 commented Oct 19, 2021

@tisonkun So, if I'm understanding right, we do setup once in a test and then use t.Run and run sub tests?

@tisonkun
Copy link
Contributor Author

@karuppiah7890 yep. Actually it is somehow we do suite batch logic manually, but this template code is similar with

var _ = Suite(XxxSuite{})

so I'm ok with it.

@karuppiah7890
Copy link
Contributor

Cool ! I'll do that ! 👍

karuppiah7890 added a commit to karuppiah7890/issues-info that referenced this issue Oct 23, 2021
Signed-off-by: Karuppiah Natarajan <[email protected]>
@tisonkun tisonkun added the type/enhancement The issue or PR belongs to an enhancement. label Feb 17, 2022
@tisonkun
Copy link
Contributor Author

tisonkun commented Mar 2, 2022

@XuHuaiyu @hawkingrei I think we may need an expert to split tests in executor_test.go first and migrate them with a file splitting.

@hawkingrei
Copy link
Member

@tisonkun @XuHuaiyu Yes, I think so. I start to split tests.

@tisonkun
Copy link
Contributor Author

tisonkun commented Mar 8, 2022

@hawkingrei I suggest you try to migrate tests rely on testdata first, so that we can remove the legacy testdata code.

@tisonkun
Copy link
Contributor Author

@hawkingrei @karuppiah7890 I've split this issue into workable subtasks, and the skeleton is constructed in #33394 and you can refer to an example in #33402.

Welcome to take over some of the subtasks.

@tisonkun
Copy link
Contributor Author

tisonkun commented Apr 6, 2022

Accidentally closed. Reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. 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
3 participants