-
Notifications
You must be signed in to change notification settings - Fork 71
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
RHOAIENG-11156: chore(tests/containers/jupyterlab): check that the JuyterLab index.html contains the spinner code #874
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
response = requests.get(f"http://{host_ip}:{host_port}/notebook/opendatahub/jovyan") | ||
assert response.status_code == 200 | ||
assert "text/html" in response.headers["content-type"] | ||
assert 'class="pf-v6-c-spinner"' in response.text |
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.
too pitty we didn't put some testid
attribute to this our extra element against which we could check here...
"--ServerApp.quit_button=False", | ||
"""--ServerApp.tornado_settings={"user":"jovyan","hub_host":"https://opendatahub.io","hub_prefix":"/notebookController/jovyan"}"""])) | ||
try: | ||
# we changed base_url, and wait_for_readiness=True would attempt connections to / |
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.
Could you please clarify, why we need to override the default base URL here?
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.
because when jupyterlab is started with --ServerApp.base_url=/notebook/opendatahub/jovyan
, accessing / will give you 404, and you need to go to /notebook/opendatahub/jovyan to get a 200 response
and starting jupyterlab without the jupyterlab options will not give you the page with spinner, but some welcome screen where it instructs you how to set tokens to connect
I guess I could've just disabled password and ignored all the other options, but I decided to use all the options dashboard normally starts the workbench with
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.
Hm, I see now; thanks for the explanation! It would be nice to put some brief explanation into the description too, if you don't mind; but I don't insist.
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.
Based on the linked CI you provided - shall I understand it that the test is expected to pass in all of our Jupyter images despite the [comment in this jira|https://issues.redhat.com/browse/RHOAIENG-11156?focusedId=26491161&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-26491161]? Based on the description in https://issues.redhat.com/browse/RHOAIENG-18915 - shall I understand it so that the fix worked and we simply didn't propagate it fully to the current 2.17 release, is that my assumption correct? |
When I ran this on the image from our 2.17 release, the test failed for me
the pytest command above gives me
|
This means yes to my question/assumption, correct? |
…pyterLab index.html contains the spinner code
b353f1a
to
ed8f99a
Compare
Yes, at least, that's also how I understand it. |
https://issues.redhat.com/browse/RHOAIENG-11156
Description
How Has This Been Tested?
Merge criteria: