-
Notifications
You must be signed in to change notification settings - Fork 11
Use playwright for functional tests #537
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
Conversation
|
That's not a good first sign... 😢 |
| - uses: actions/upload-artifact@v4 | ||
| if: ${{ !cancelled() }} | ||
| with: | ||
| name: playwright-traces | ||
| path: build/test-results/ |
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.
Uploads traces you can view at https://trace.playwright.dev/ if tests fail
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 exceptionally cool - do you mind documenting it somewhere in the repo?
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.
Added documentation!
| $('#current-uii').text(deptUiidLabel); | ||
| $('#uii-question').show(); | ||
| } else { | ||
| $('#unique_internal_identifier').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.
Fixed a minor bug where unique internal identifier would still be included in the form submission if you fill in the field for a department with UII but then switched to a department without UII
| - ./poetry.lock:/usr/src/app/poetry.lock | ||
| - ./pyproject.toml:/usr/src/app/pyproject.toml | ||
| - ./OpenOversight:/usr/src/app/OpenOversight | ||
| - ./build/test-results:/test-results |
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.
Mount the folder with Playwright traces to build/test-results
| poetry install --only main --no-root; \ | ||
| else \ | ||
| poetry install --no-root; \ | ||
| playwright install --with-deps chromium; \ |
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.
Switching functional tests from Firefox to Chromium due to current browser market shares
AetherUnbound
left a comment
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 fantastic! What a simplification! I'm glad we could also get the pytest.ini settings into the pyproject.toml 😄
| - uses: actions/upload-artifact@v4 | ||
| if: ${{ !cancelled() }} | ||
| with: | ||
| name: playwright-traces | ||
| path: build/test-results/ |
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 exceptionally cool - do you mind documenting it somewhere in the repo?
Dockerfile
Outdated
| # Install packages depending on ENV arg from docker-compose | ||
| ARG BASE_PACKAGES="gcc libpq-dev libjpeg62-turbo-dev libsqlite3-0 zlib1g-dev" | ||
| ARG DEV_PACKAGES="firefox-esr" | ||
| ARG DEV_PACKAGES="" |
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.
If we're dropping that package, I think we can remove all of the differentiation between packages to install per environment in the Dockerfile!
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.
Removed extra Dockerfile logic!
Description of Changes
Switch to Playwright for functional tests. The Playwright API seems easier to use in many ways than Selenium
Notes for Deployment
None! This only applies to tests
Screenshots (if appropriate)
N/A
Testing instructions
Run
just testChecks
I have rebased my changes on
mainjust lintpassesjust testpasses