Skip to content
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 e2e prerender-related tests #2630

Open
wants to merge 21 commits into
base: next
Choose a base branch
from
Open

Add e2e prerender-related tests #2630

wants to merge 21 commits into from

Conversation

csjh
Copy link
Contributor

@csjh csjh commented Oct 8, 2024

Description

Closes #2581

Adds tests for chart double loading, dropdown prerendering, initialData usage in page/input/manual queries, etc

a lot more can be added (i.e. one for each input, bigvalue test, etc) but I think this is a good start

Checklist

Copy link

changeset-bot bot commented Oct 8, 2024

🦋 Changeset detected

Latest commit: ddf3be8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@evidence-dev/component-utilities Patch
@evidence-dev/core-components Patch
my-evidence-project Patch
@evidence-dev/components Patch
evidence-test-environment Patch
e2e-prerender Patch
e2e-spa Patch
e2e-themes Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Oct 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 14, 2024 6:40pm
next-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 14, 2024 6:40pm

Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for evidence-test-env ready!

Name Link
🔨 Latest commit ddf3be8
🔍 Latest deploy log https://app.netlify.com/sites/evidence-test-env/deploys/670d634ee3127800087dcb91
😎 Deploy Preview https://deploy-preview-2630--evidence-test-env.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for evidence-development-workspace ready!

Name Link
🔨 Latest commit ddf3be8
🔍 Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/670d634e84c25700080ceadb
😎 Deploy Preview https://deploy-preview-2630--evidence-development-workspace.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 8, 2024

Deploy Preview for next-docs-evidence ready!

Name Link
🔨 Latest commit ddf3be8
🔍 Latest deploy log https://app.netlify.com/sites/next-docs-evidence/deploys/670d634e31eaf300081a7786
😎 Deploy Preview https://deploy-preview-2630--next-docs-evidence.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

.changeset/dry-bobcats-beg.md Outdated Show resolved Hide resolved
.changeset/dry-bobcats-beg.md Outdated Show resolved Hide resolved
e2e/prerender/playwright.config.js Outdated Show resolved Hide resolved
e2e/prerender/tests/tests.spec.js Outdated Show resolved Hide resolved
@@ -11,7 +11,7 @@ import * as chartWindowDebug from './chartWindowDebug';
* } ActionParams
*/

const ANIMATION_DURATION = 500;
const ANIMATION_DURATION = navigator.webdriver ? 0 : 500;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this disabling animations in tests? If so, I don't think we should make this change. Testing in general is most effective when the code we're testing matches the code users are running. Adding differences like this reduces the effectiveness of testing.

If you added this to avoid adding delays to the tests, instead you should look for another way to know that the chart is fully mounted/animated (like a callback/flag) if that is important for the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did that so I could use the finished event to figure out chart rerenders - there's a rendered event I'm trying but it seems like it might not be very consistent

@ItsMeBrianD
Copy link
Member

@zachstence to check on why e2e tests aren't running here

@zachstence
Copy link
Member

@zachstence to check on why e2e tests aren't running here

Seems fixed by resolving merge conflicts


// 3 renders is what is expected (creation -> update -> (deletion) -> creation)
// double loading makes 4 renders, adding another delete/create in
// TODO: why is 3 renders expected
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this TODO?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

e2e tests for validating prerendering/lack of double loading
3 participants