-
Notifications
You must be signed in to change notification settings - Fork 682
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
E2e test in k8s cluster and Namespace option #1774
E2e test in k8s cluster and Namespace option #1774
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Pull Request Test Coverage Report for Build 4424758153
💛 - Coveralls |
@nagar-ajay Can you do a rebase as #1775 is merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nagar-ajay Thank you for creating this PR! I left a few comments.
44d5caa
to
f59c15f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
/lgtm
/assign @johnugeorge
Thanks @nagar-ajay /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnugeorge, nagar-ajay 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 |
What this PR does / why we need it:
Currently, the
JOB_NAMESPACE
is hardcoded in e2e test files. This limits a user to run these tests in thedefault
namespace. A user might want to run these tests in a different namespace instead of the default namespace. One use case is using these tests in Kubeflow mode for training-operator conformance tests (or some other testing). As per the design doc of the conformance test, it seems we want to run these tests inkf-conformance
namespace.Currently, the training client initialization code looks for Kube config which may not be present if these tests are running inside k8s cluster. Fix training client initialization code to run inside and outside of k8s cluster.
Created
conftest.py
for pytest fixtures. Make job namespace a fixture. After this PR, users can pass the--namespace
option from the command line to run e2e tests in a specific namespace.As part of
hack/python-sdk/gen-sdk.sh
all the python files under the test directory gets deleted. We need to delete only autogenerated test files, not every file. Updated the code in thegen-sdk.sh
file to delete python files that start withtest_
.Testing: Tested modified tests on the NKE cluster from local.
Which issue(s) this PR fixes (optional, in
Fixes #<issue number>, #<issue number>, ...
format, will close the issue(s) when PR gets merged):Fixes #
Checklist: