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

Add e2e test for the component build(use multiple output sample) #2109

Closed
gaoning777 opened this issue Sep 13, 2019 · 8 comments · Fixed by #2147
Closed

Add e2e test for the component build(use multiple output sample) #2109

gaoning777 opened this issue Sep 13, 2019 · 8 comments · Fixed by #2147

Comments

@gaoning777
Copy link
Contributor

This issue is to track adding a sample test for the component build(func_to_containerop).
Currently we have a sample https://github.com/kubeflow/pipelines/tree/master/samples/core/multiple_outputs, which uses the build_python_component function. We can convert it to use the new func_to_containerop and add the test to the e2e.

@gaoning777 gaoning777 changed the title Add e2e test for the component build Add e2e test for the component build(use multiple output sample) Sep 13, 2019
@gaoning777
Copy link
Contributor Author

gaoning777 commented Sep 17, 2019

@gaoning777
Copy link
Contributor Author

#1813

@gaoning777
Copy link
Contributor Author

multiple_output sample is not tested, yet

@Ark-kun
Copy link
Contributor

Ark-kun commented Sep 23, 2019

multiple_output sample is not tested, yet

Tracking that in #2208
Also #2209 #2210

@gaoning777
Copy link
Contributor Author

gaoning777 commented Sep 24, 2019

Is the func_to_container_op conversion necessary for the tests?
Or we could add the tests and then convert?

@Ark-kun
Copy link
Contributor

Ark-kun commented Sep 30, 2019

Is the func_to_container_op conversion necessary for the tests?

It's not necessary. I was just going with what you put in the description of this issue: "We can convert it to use the new func_to_containerop and add the test to the e2e.".

@gaoning777
Copy link
Contributor Author

SGTM. Let's add the test first then.

@Ark-kun
Copy link
Contributor

Ark-kun commented Oct 22, 2019

@gaoning777 Should we close this issue since both samples are now covered by sample tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants