-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add Docs Tests #2502
Add Docs Tests #2502
Conversation
43ee1f9
to
cb5750c
Compare
We've added HTML reporting to this PR. This allows us to download an artifact containing the results of the test. |
a253939
to
fb317d0
Compare
4e9dab0
to
679cffa
Compare
Another update: |
co-authored by: Ada Mandala <[email protected]>
ffb1674
to
2cb4073
Compare
fix broken svg comparisons
2cb4073
to
9cd4eec
Compare
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.
Thanks for the PR! Looks good!
@@ -157,7 +161,7 @@ export default defineConfig({ | |||
forbidOnly: !!process.env.CI, | |||
retries: process.env.CI ? 2 : 0, | |||
quiet: true, | |||
reporter: process.env.CI ? "github" : "dot", | |||
reporter: process.env.CI ? [["github"], ["html"]] : [["dot"]], |
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.
?
@@ -49,6 +49,11 @@ async function run_with_theme(page, is_dark = false) { | |||
await page.goto("http://localhost:8080/"); | |||
await page.setContent(template(is_dark)); | |||
await page.setViewport(DEFAULT_VIEWPORT); | |||
await page.evaluate(async () => { | |||
while (!window.__TEST_PERSPECTIVE_READY__) { |
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 __TEST_PERSPECTIVE_READY__
were a Promise
, we could just await
it here.
await page.evaluate(async (config) => { | ||
const viewer = document.querySelector("perspective-viewer"); | ||
await viewer.reset(); | ||
await viewer.restore(config); | ||
}, new_config); | ||
|
||
await page.waitForSelector("perspective-viewer:not([updating])"); |
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.
What is the story behind this + lines 52-56 in this file?
with: | ||
name: playwright-report | ||
path: tools/perspective-test/playwright-report/ | ||
retention-days: 30 |
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 will keep an eye on the impact of these artifacts on our GitHub footprint!
Currently on https://perspective.finos.org, there are broken examples on the home page. This occurs on X/Y scatter plots due to a failing migration.
Building and testing on main does not replicate the bug, as the website is running on v2.7.1 and a fix has been added since that release.
This PR simply adds tests for the examples (one for each example, totaling 75) to ensure this doesn't happen again.