Adding integrations test for the CLI commands related to pipelines. #125
Conversation
|
|
||
| // Create pipeline | ||
| args := []string{"pipeline", "create", | ||
| "https://storage.googleapis.com/ml-pipeline-dataset/sequential.yaml"} |
There was a problem hiding this comment.
can you copy the file in resources/ folder and refer it there? it makes test more self contained, and reduce test flakiness.
There was a problem hiding this comment.
This "pipeline create" call is supposed to take a URL as a parameter.
The "pipeline upload" call is testing uploading a local file.
Is there a way to use "pipeline create" with a local file?
There was a problem hiding this comment.
I was thinking about the other API. my bad. please ignore.
backend/test/cli_test.go
Outdated
| } | ||
|
|
||
| func (c *CLIIntegrationTest) deletePipelines() error { | ||
|
|
| assert.Nil(t, err) | ||
| } | ||
|
|
||
| func (c *CLIIntegrationTest) TestPipelineCreateGetDeleteSuccess() { |
There was a problem hiding this comment.
i see you called List in delete step, but maybe have a section below to test list explicitly, and add some assertions against the list results too?
There was a problem hiding this comment.
There is a TestPipelineListSuccess above.
For the test organization, I assuming the following:
- The API server e2e tests will check that the clients return the correct results.
- The CLI unit tests verify that what is returned by the API server is correctly converted to the CLI output.
- These tests simply verify that the connection between the CLI and API server works.
As an alternative, if you are open to it, we could delete the API server e2e tests and rely on the CLI e2e tests for all the API tests... It seems that this would save a lot of work and make sure we deliver CLI updates quickly. WDYT?
There was a problem hiding this comment.
Added a check on the "pipeline list" output:
- checked that the resulting pipelines can be parsed
- checked that there is at least one of them.
|
/lgtm |
|
/retest |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vicaire 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 |
|
/lgtm |
…kubeflow#126) show up in build-log.txt reported to Gubernator. Related to kubeflow#125
feat(backend): Add metadata field to scheduled workflow
This change is