-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Test/Server code out of sync, blocking PRs #196
Comments
That issue got fixed when I explicitly re-run build-image step for all problematic PRs. I do not fully understand how the unit test image has caused problem, since it should only contain the |
I think these are integration test failures. The issue might be that your build-image step is ran before #112 and the test is ran after that PR. The test and server code are out of sync. Thus, I don't think we should have this change #165 Please consider revert the change. |
(The test code would use client library which is par of the source code) |
Turns out #165 does not fix the issue completely, but it's at least closer to what we should ideally have. Ideally, all our tests should test the code that Prow prepared before running the test. Without #165, the desync happens in ~90% of cases. With #165 it happens in ~10% of cases. Without #165, the desync is unfixable - the tests will never succeed unless you merge master to the branch (which removes all LGTM and APPROVED labels). With #165 the bad desync can be fixed by re-running the build-image step. The only way to 100% fix this is to never do |
Here is how to fix this for backend unit tests:
|
…eflow#195) * Only consider include_dirs on presubmit tests. * We all tests to be triggered only when certain files are modified. * I think it only makes sense to apply this filter on presubmits; for postsubmits we should include always run a test. Related to: kubeflow/kubeflow#1048 * update readme. * Try to fix kubeflow#196 by updating funcsigs.
https://gubernator.k8s.io/build/kubernetes-jenkins/pr-logs/pull/kubeflow_pipelines/173/presubmit-e2e-test/87
https://gubernator.k8s.io/build/kubernetes-jenkins/pr-logs/pull/kubeflow_pipelines/168/presubmit-e2e-test/89
https://gubernator.k8s.io/build/kubernetes-jenkins/pr-logs/pull/kubeflow_pipelines/148/presubmit-e2e-test/105
https://gubernator.k8s.io/pr/kubeflow/pipelines/148
https://gubernator.k8s.io/pr/kubeflow/pipelines/172
https://gubernator.k8s.io/pr/kubeflow/pipelines/173
The text was updated successfully, but these errors were encountered: