search

kubeflow / pipelines

Machine Learning Pipelines for Kubeflow
https://www.kubeflow.org/docs/components/pipelines/
Apache License 2.0
3.62k stars 1.63k forks source link

fix(tests): remove redundant integration test wf #11322

Closed droctothorpe closed 1 month ago

droctothorpe commented 1 month ago

Description of your changes: Identical sample tests are being run in two separate CI workflows for some reason: kfp-samples and kubeflow-pipelines-integration-v2. The first invokes sample_test.py. The second does as well (after pathing through a Makefile). Presumably, we don’t need both.

I suggest we tackle eliminating integration-test.sh and some of the corresponding out of date docs in a follow-up PR. There should be one, clearly documented path for running large tests both in CI and from local.

Checklist:

droctothorpe commented 1 month ago

/ok-to-test

VaniHaripriya commented 1 month ago

/lgtm

droctothorpe commented 1 month ago

Thank you for addressing my suggestions @droctothorpe.

I suggest we tackle eliminating integration-test.sh

@droctothorpe can you remove the file in this PR.

You probably missed this comment. Can we remove the file in this PR if it doesn't impact anything?

Done! 👍

rimolive commented 1 month ago

Based on @hbelmiro suggestions that was already addressed, I believe we're good to go.

/lgtm

HumairAK commented 1 month ago

/approve

google-oss-prow[bot] commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: HumairAK

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: - ~~[.github/OWNERS](https://github.com/kubeflow/pipelines/blob/master/.github/OWNERS)~~ [HumairAK] - ~~[backend/OWNERS](https://github.com/kubeflow/pipelines/blob/master/backend/OWNERS)~~ [HumairAK] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
chensun commented 1 month ago

Just to share a bit history, in the past, sample tests and integration tests are different workflows, IIRC, the former runs against the latest release--we do this by maintaining a long running deployment and update it post every release--while the latter runs against the built from master head + the PR being tested--developing on the fly. I sense this current GA workflow may have merged, but in the long run, I think there're value to test with both stable (release) and branch head.