RHOAIENG-39075: Add new E2E test structure and initial existing cluster test #948
Conversation
|
@kryanbeane: This pull request references RHOAIENG-39075 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
kryanbeane
force-pushed
the
RHOAIENG-39075
branch
from
6f56f55 to
5376b68
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #948 +/- ##
==========================================
- Coverage 94.21% 94.08% -0.14%
==========================================
Files 24 24
Lines 2128 2130 +2
==========================================
- Hits 2005 2004 -1
- Misses 123 126 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pawelpaszki 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 |
openshift-ci
bot
added
the
approved
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
|
|
||
| ```bash | ||
| # Run all pre-upgrade tests including UI tests | ||
| poetry run pytest tests/upgrade/ -m pre_upgrade -v |
There was a problem hiding this comment.
Given there are some options here what do we expect to run on prs? Or is this something we will do manually first as part of upgrade testing?
| if "/ray/" in dashboard_url: | ||
| # HTTPRoute format: https://hostname/ray/namespace/cluster-name | ||
| # API endpoint is at the same base path | ||
| api_url = dashboard_url + "/api/jobs/" |
There was a problem hiding this comment.
Should this have the /namespace/clustername in it?
|
|
||
| @pytest.mark.pre_upgrade | ||
| @pytest.mark.ui | ||
| class TestDistributedWorkloadsUIPreUpgrade: |
There was a problem hiding this comment.
did you find much difference between the pre and post upgrade steps. And also in terms of maintaining them, given there could be changes to where the Clusters/Jobs are represented, do we have a ticket to track that change? Or is it that we'd expect the tests to fail and then fix?
|
|
||
|
|
||
| # Creates a Ray cluster , submit RayJob mnist script long running | ||
| class TestSetupSleepRayJob: |
There was a problem hiding this comment.
What is the difference between this and the previous test of mnist job submission?
laurafitzgerald
left a comment
laurafitzgerald
left a comment
There was a problem hiding this comment.
Hi @kryanbeane a few comments inline. Overall the change looks good. I didn't review the utils files in anger but please let me know if i should. This gives us a great structure going forward for adding the additional testing. Will those be done in seperate prs?
Yep separate PRs. This PR is mostly to decide on structure. Pawel pointed out we never got rid of AppWrapper, so I'll follow up with that PR. Then I'll create new versions of our old tests in this new layout, last will be adding additional tests |
kryanbeane
force-pushed
the
RHOAIENG-39075
branch
from
5376b68 to
955c37f
Compare
|
New changes are detected. LGTM label has been removed. |
4 participants
Issue link
https://issues.redhat.com/browse/RHOAIENG-39075
What changes have been made
Added new E2E directory structure
Verification steps
N/A
Checks