[Backend] Refactor integration tests, facilitate local testing #3138
Conversation
|
/assign @IronPan |
| @@ -207,12 +216,6 @@ func (s *JobApiTestSuite) TestJobApis() { | |||
| assert.Equal(t, 1, totalSize) | |||
| argParamsRun := runs[0] | |||
| s.checkArgParamsRun(t, argParamsRun, argParamsExperiment.ID, argParamsExperiment.Name, argParamsJob.ID, argParamsJob.Name) | |||
|
|
|||
There was a problem hiding this comment.
My understanding is that you make sure all the pipelines/runs/experiments/... are cleaned up before the test begins in SetupTests() methods, and therefore you don't need to clean up those pipelines/runs/experiments/... after the test ends, which enables the dev to debug integration test errors after the test ends. Is that correct? If so, can we add comments here as why we don't clean up after test ends (since conventionally we do)?
There was a problem hiding this comment.
Yes, that's correct.
Another benefit is, if you halt your test in the middle, rerunning the test won't fail because of cluster in an intermediate state.
There was a problem hiding this comment.
I just realized, frontend-integration-test actually fails after this.
So I guess either we need to add cluster cleanup in frontend-integration-test, or we still need clean up after each integration test.
There was a problem hiding this comment.
Updated implementation to use a flag to turn off resource clean up when developing it.
| } | ||
|
|
||
| func TestExperimentAPI(t *testing.T) { | ||
| suite.Run(t, new(ExperimentApiTest)) | ||
| } | ||
|
|
||
| func (s *ExperimentApiTest) TearDownSuite() { |
There was a problem hiding this comment.
Is this method used in manual process? Didn't see it called in the script.
There was a problem hiding this comment.
https://godoc.org/github.com/stretchr/testify/suite#TearDownAllSuite
it will be called by testify
There was a problem hiding this comment.
I see. Thanks!
/lgtm
/approve
|
Other than a nit question regarding TearDownSuite(), /lgtm and /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jingzhang36 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
New changes are detected. LGTM label has been removed. |
|
@jingzhang36 thanks for the review! |
…low#3138) * Make local testing easier * Move cleanup to test setup stage * Add readme for how to run integration tests * Add warning about data loss * Add warning also in the script * Change flag to isDevMode and cleanup resources if not in dev mode * Pass through arguments in the bash script * Fix unit tests
Add README and script to help run backend integration tests locally.
Also moved cleanup steps to test setup stage, so that
UPDATE: tests will clean up both before and after it runs, to unblock frontend-integration-test which depends on the cluster being empty, but clean up after test is disabled in dev mode
This change is