-
Notifications
You must be signed in to change notification settings - Fork 21
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
Fix playwright tests #372
Fix playwright tests #372
Conversation
@@ -118,7 +118,7 @@ def _create_new_environment(page, screenshot=False): | |||
|
|||
# Interact with the environment shortly after creation | |||
# click to open the Active environment dropdown manu | |||
page.get_by_role("button", name=" - Active", exact=False).click() | |||
page.get_by_text(" - Active", exact=False).click() |
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.
This is necessary because the generated Select
component is now a combobox
, so the button
role doesn't match.
Using get_by_text
might look error-prone here, but I think it's okay. The option
line below will error out if get_by_text
matches something else 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.
Note: I use get_by_text
and not get_by_role("combobox", ...)
here because playwright fails to find this button with the latter approach.
b464514
to
c0d594a
Compare
c0d594a
to
facc942
Compare
@@ -193,7 +193,7 @@ def _existing_environment_interactions(page, env_name, time_to_build_env=3*60*10 | |||
# change the description | |||
page.get_by_placeholder("Enter here the description of your environment").fill("new description") | |||
# change the vesion spec of an existing package | |||
page.get_by_role("row", name="ipykernel", exact=False).get_by_role("button").first.click() | |||
page.get_by_role("row", name="ipykernel", exact=False).get_by_role("combobox").first.click() |
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.
As mentioned in the other comment, the generated code now uses combobox
instead of button
.
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.
When I was reviewing this PR, I noticed that after running yarn start
if I loaded the web app from localhost:8000, I got a combobox, whereas if I loaded the web app from localhost:8080 I got a button.
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.
Okay, I think I see why this happens. It looks like the Conda Store server comes bundled with its own version of Conda Store UI. Wild.
So then this change makes sense; however, I'm now concerned about a possible discrepancy between the environment variables in the GitHub runner environment versus local dev, resulting in this test passing on CI but failing locally.
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.
We might need to redefine the default value for CONDA_STORE_SERVER_PORT
at the top of the file.
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.
Wait, when was this changed to a combobox? - I thought this was during the #310 PR
In that case both 8080
and 8000
should be returning a combobox.
@@ -89,7 +89,7 @@ def _create_new_environment(page, screenshot=False): | |||
# ensure new filename in case this test is run multiple times | |||
new_env_name = f'test_env_{random.randint(0, 100000)}' | |||
# set timeout for building the environment | |||
time_to_build_env = 2 * 60 * 1000 # 2 minutes in milliseconds | |||
time_to_build_env = 5 * 60 * 1000 # 5 minutes in milliseconds |
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.
The previous timeout value wasn't big enough, so the test was flaky.
This reverts commit 37cf133.
Could you please explain the process you used to update the yarn lockfile in detailed steps? As it stands, I don't know how to recreate this PR. |
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.
I'm marking this as request changes because I'm blocked on reviewing this PR on the following two things:
- I need to know how to generate the lockfile changes. (See my earlier comment.)
- I need to know how to run the tests locally. Here's a PR showing the steps I followed to try to run the tests locally.
@@ -193,7 +193,7 @@ def _existing_environment_interactions(page, env_name, time_to_build_env=3*60*10 | |||
# change the description | |||
page.get_by_placeholder("Enter here the description of your environment").fill("new description") | |||
# change the vesion spec of an existing package | |||
page.get_by_role("row", name="ipykernel", exact=False).get_by_role("button").first.click() | |||
page.get_by_role("row", name="ipykernel", exact=False).get_by_role("combobox").first.click() |
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.
When I was reviewing this PR, I noticed that after running yarn start
if I loaded the web app from localhost:8000, I got a combobox, whereas if I loaded the web app from localhost:8080 I got a button.
@gabalafou Happy to help, LMK if the following doesn't answer your questions.
I did
playwright_env.sh:
diff --git a/test/playwright/test_ux.py b/test/playwright/test_ux.py
index dcf0782..ab7c42f 100644
--- a/test/playwright/test_ux.py
+++ b/test/playwright/test_ux.py
@@ -306,7 +306,7 @@ if __name__ == "__main__":
# Use playwright.chromium, playwright.firefox or playwright.webkit
# Pass headless=False to launch() to see the browser UI
# slow_mo adds milliseconds to each playwright command so humans can follow along
- browser = playwright.chromium.launch(headless=False, slow_mo=500)
+ browser = playwright.chromium.launch(headless=True, slow_mo=500)
page = browser.new_page()
# Go to http://localhost:{server_port}
I'd also add that these tests are passing on CI. So if there are any problems, make sure you have no other instances of conda-store running, etc. |
Fixes playwright test failures introduced by a yarn update.
Description
This pull request:
Select
elements, which are generated with rolecombobox
after the lockfile update. For example, this shows the change to the code of theActive
button:Pull request checklist
Additional information