Fixed #37154 -- Switched from Selenium to Playwright for integration testing.#7
Fixed #37154 -- Switched from Selenium to Playwright for integration testing.#7varunkasyap wants to merge 1 commit into
Conversation
| run: python -m pip install --upgrade pip wheel | ||
| - run: python -m pip install -r tests/requirements/py3.txt -e . | ||
| - name: Install Playwright browsers | ||
| run: python -m playwright install --with-deps chromium |
There was a problem hiding this comment.
Playwright docs on CI:
https://playwright.dev/python/docs/ci#:~:text=On%20push/pull_request
|
In Playright: Arguments
example-
Optional load state to wait for, defaults to load. If the state has been already reached while loading current document, the method resolves immediately. Can be one of: 'load' - wait for the load event to be fired. |
|
In playwright: expect() is used for assetions reference - https://playwright.dev/python/docs/test-assertions#list-of-assertions |
|
Playwright uses milliseconds, whereas, selenium used seconds |
| cls._old_async_unsafe = os.environ.get("DJANGO_ALLOW_ASYNC_UNSAFE") | ||
| os.environ["DJANGO_ALLOW_ASYNC_UNSAFE"] = "true" |
There was a problem hiding this comment.
otherwise, getting this error
File "C:\Users\varun\OneDrive\Desktop\os\django\django\utils\asyncio.py", line 24, in inner
raise SynchronousOnlyOperation(message)
django.core.exceptions.SynchronousOnlyOperation: You cannot call this from an async context - use a thread or sync_to_async.
| cls._browser_context = cls._browser.new_context() | ||
| cls._browser_logs = [] | ||
| cls.page = cls._browser_context.new_page() | ||
| # Set up CDP session for browser-level logs (Chromium only). |
There was a problem hiding this comment.
temporary workaround for now, TODO - should make this compatible for all browsers.
sarahboyce
left a comment
There was a problem hiding this comment.
Thank you!
I've added a few suggestions from an initial look at the tests
| if ( | ||
| multiprocessing.get_start_method() in {"spawn", "forkserver"} | ||
| and options.parallel != 1 | ||
| ): | ||
| parser.error( | ||
| "You cannot use --playwright with parallel tests on this system. " | ||
| "Pass --parallel=1 to use --playwright." | ||
| ) |
There was a problem hiding this comment.
| if ( | |
| multiprocessing.get_start_method() in {"spawn", "forkserver"} | |
| and options.parallel != 1 | |
| ): | |
| parser.error( | |
| "You cannot use --playwright with parallel tests on this system. " | |
| "Pass --parallel=1 to use --playwright." | |
| ) |
I can remove this without errors. I think ideally our tests don't need to have a shared state so would be interested if we can have this be removed and only added if really needed
| options.tags = ["playwright"] | ||
| elif "playwright" not in options.tags: | ||
| options.tags.append("playwright") | ||
| # TODO VK external_host and playwright_hub ?? |
There was a problem hiding this comment.
I don't think we will need a playwright_hub
| parser.add_argument( | ||
| "--playwright-headless", | ||
| action="store_true", | ||
| help="Run Playwright tests in headless mode, if the browser supports " | ||
| "the option.", | ||
| ) |
There was a problem hiding this comment.
I think let's reuse the existing headless option
| self.page.click(f"input[value='{login_text}']") | ||
| self.page.wait_for_load_state("load") |
There was a problem hiding this comment.
| self.page.click(f"input[value='{login_text}']") | |
| self.page.wait_for_load_state("load") | |
| with self.page.expect_navigation(): | |
| self.page.get_by_role("button", name=login_text).click() |
Perhaps?
There was a problem hiding this comment.
Thanks for the review @sarahboyce
expect_navigation() seems depricated
Playwight is suggesting to use page.wait_for_url() instead.
so,
| self.page.click(f"input[value='{login_text}']") | |
| self.page.wait_for_load_state("load") | |
| self.page.get_by_role("button", name=login_text).click() | |
| self.page.wait_for_url(f"{self.live_server_url}{login_url}") |
Perhaps?
There was a problem hiding this comment.
I think we can have expect(page).to_have_url(...) and I think the url is the admin index page
I'm not sure what's best practice really, it would be good for us to agree what we should do in most navigation cases
| size = self.page.viewport_size | ||
| self.page.set_viewport_size( | ||
| {"width": size["width"] + 1, "height": size["height"]} | ||
| ) | ||
| self.page.wait_for_load_state("domcontentloaded") | ||
| self.page.set_viewport_size(size) | ||
| self.page.wait_for_load_state("domcontentloaded") |
There was a problem hiding this comment.
| size = self.page.viewport_size | |
| self.page.set_viewport_size( | |
| {"width": size["width"] + 1, "height": size["height"]} | |
| ) | |
| self.page.wait_for_load_state("domcontentloaded") | |
| self.page.set_viewport_size(size) | |
| self.page.wait_for_load_state("domcontentloaded") | |
| self.page.evaluate( | |
| "window.dispatchEvent(new Event('resize'))" | |
| ) |
And perhaps the selenium version should be:
self.selenium.execute_script(
"window.dispatchEvent(new Event('resize'));"
)I also think we barely use this so wondering if it is really worth being a helper method 🤔
| self.page.click(f"input[value='{login_text}']") | ||
| self.page.wait_for_load_state("load") | ||
|
|
||
| def select_option(self, selector, value): |
There was a problem hiding this comment.
As agreed, we should decide if each of these helpers are needed
| is_valid = self.page.evaluate( | ||
| "document.getElementById('id_number').checkValidity()" | ||
| ) |
There was a problem hiding this comment.
| is_valid = self.page.evaluate( | |
| "document.getElementById('id_number').checkValidity()" | |
| ) | |
| is_valid = number_input.evaluate("el => el.checkValidity()") |
Perhaps
|
|
||
| @tag("playwright") | ||
| class PlaywrightTestCase(LiveServerTestCase, metaclass=PlaywrightTestCaseBase): | ||
| default_timeout = 10000 # milliseconds |
There was a problem hiding this comment.
Is the default_timout logic required? Not sure if we were hacking in things for selenium which playwright solves for us
| self.page.locator("#submit").click() | ||
| self.page.wait_for_load_state("load") |
There was a problem hiding this comment.
| self.page.locator("#submit").click() | |
| self.page.wait_for_load_state("load") | |
| with self.page.expect_navigation(): | |
| self.page.locator("#submit").click() |
| from playwright.sync_api import expect | ||
|
|
||
| self.page.goto(self.live_server_url + "/jsi18n_template/") | ||
| self.page.wait_for_load_state("domcontentloaded") |
52b738f to
4144053
Compare
4144053 to
342dae8
Compare
342dae8 to
c4a0493
Compare
| # Confirm they're selected after clicking inactive buttons: ticket | ||
| # #26575 | ||
| self.assertSelectedOptions(from_box, [str(self.peter.id), str(self.lisa.id)]) | ||
| self.page.locator(remove_button).click(force=True) |
There was a problem hiding this comment.
The Selenium test intentionally clicks disabled buttons
Playwright refuses to click disabled elements by default. Hence, added force=True
c4a0493 to
2884c14
Compare
|
|
||
| def setUp(self): | ||
| if self.browser == "webkit": | ||
| self.skipTest("WebKit Tab key only focuses form controls, not links.") |
There was a problem hiding this comment.
Safari User Guide says:
Highlight the next field or pop-up menu on a web page: Tab
Highlight the next field, pop-up menu or clickable item (such as a link) on a web page: Option-Tab
| # `Skip link` is not present. | ||
| skip_link = self.page.locator(".skip-to-content-link") | ||
| self.assertFalse( | ||
| skip_link.evaluate("el => el.getBoundingClientRect().top >= 0") |
There was a problem hiding this comment.
The skip link hides via top: -999px CSS, which is off-screen but Playwright's is_visible() still considers it visible. Checking getBoundingClientRect().top >= 0 can detects whether the element is on-screen.
alternatives:
• self.assertFalse(...) → expect(skip_link).not_to_be_in_viewport()
• self.assertTrue(...) → expect(skip_link).to_be_in_viewport()
2575577 to
07f99f2
Compare
f3415ea to
68477c4
Compare
82018a6 to
ae8d708
Compare
8cdb706 to
da7fdf5
Compare
a9b85d9 to
b287baf
Compare
942774b to
e27b977
Compare
e27b977 to
cfbafac
Compare
cfbafac to
d902339
Compare
Notes:
so, Extracting the Tag Name from an Element tag_name selenium possible
example
due to architecture change in Playwright, it doesn't natively expose DOM properties.
playwright translation looks like