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 other executor tests to K8s tests #47472

Conversation

jscheffl
Copy link
Contributor

@jscheffl jscheffl commented Mar 6, 2025

Somehow ONLY k8sExecutor used pytests in K8s CI deployments. @potiuk pointed me to the fact that other executors also had tests... but seems they were not called.

This PR (attempts) to add the tests for other executors

@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

As discussed #46502 was accidentally removing the other tests in Feb 😓 ..

@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

We should bring back all tests.

@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

There are also KPO tests for example.

@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

cc: @jason810496 -> Maybe you can bring it back, apparently I missed that #46502 only run k8S test.

@jscheffl
Copy link
Contributor Author

jscheffl commented Mar 6, 2025

As #46502 was made to remove flakiness... hopefully this does not add it back again :-O

@potiuk
Copy link
Member

potiuk commented Mar 6, 2025

👀

@jason810496
Copy link
Member

I accidentally committed the modified test case (I only ran the KubernetesExecutor locally for faster iteration during development).

I believe this won’t reintroduce flakiness, as I simply replaced time.sleep with kubectl rollout status --watch.

@jscheffl jscheffl force-pushed the bugfix/add-k8s-tests-in-breeze-for-other-executors branch 2 times, most recently from da1deaa to cbc4fa4 Compare March 7, 2025 13:19
@jscheffl jscheffl force-pushed the bugfix/add-k8s-tests-in-breeze-for-other-executors branch from cbc4fa4 to 28aeeb0 Compare March 7, 2025 21:47
@jscheffl
Copy link
Contributor Author

jscheffl commented Mar 7, 2025

Green (besides sadly I needed to add XFail for the LocalExecutor tests which are caused by not able to make LocalExecutor work in KinD --> #47518 )

@potiuk potiuk merged commit 687b867 into apache:main Mar 8, 2025
89 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants