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

feat: playwright tests #64

Merged
merged 58 commits into from
Nov 14, 2023
Merged

feat: playwright tests #64

merged 58 commits into from
Nov 14, 2023

Conversation

hirasso
Copy link
Member

@hirasso hirasso commented Nov 13, 2023

Description

Adds basic e2e test coverage to the plugin.

Checks

  • The PR is submitted to the master branch
  • The code was linted before pushing (npm run lint)
  • All tests are passing (npm run test)
  • New or updated tests are included

@hirasso hirasso requested a review from daun November 14, 2023 07:35
@hirasso
Copy link
Member Author

hirasso commented Nov 14, 2023

TIL caching playwright isn't necessarily speeding up actions.

Without cache: 2m 20s
With cache: 2m 57s

@hirasso
Copy link
Member Author

hirasso commented Nov 14, 2023

@daun do you understand what's happening here?

https://github.com/swup/fragment-plugin/actions/runs/6860973311/job/18655736640?pr=64#step:8:10

Error: Report file results.json not found. Make sure Playwright is configured to generate a JSON report.

If I understand the config correctly, it should spit out the results.json at the root when in CI:

reporter: process.env.CI
	? [['dot'], ['github'], ['json', { outputFile: '../../report.json' }]]
	: [['list'], ['html', { outputFolder: '../reports/html', open: 'on-failure' }]],

Although swup's playwright config doesn't even report JSON at all when in CI: https://github.com/swup/swup/blob/1cd87ea335b95b9b8caee602634974f5011d2cc7/tests/config/playwright.config.ts#L32-L40

@hirasso
Copy link
Member Author

hirasso commented Nov 14, 2023

It's not THAT important I'd say, I could totally live without the reporter in the PR tbh :)

@daun
Copy link
Member

daun commented Nov 14, 2023

@hirasso It's currently report.json vs. results.json 🤠 Matching both should fix it hopefully.

@hirasso
Copy link
Member Author

hirasso commented Nov 14, 2023

Haha true 😆 so many trees 🌲🌳🌴🏝️

Copy link

github-actions bot commented Nov 14, 2023

Playwright test results

passed  12 passed

Details

stats  12 tests across 1 suite
duration  28.7 seconds
commit  b2332df

@hirasso
Copy link
Member Author

hirasso commented Nov 14, 2023

OMG it's working! 🤯🥳

...ready for review 🤪

@hirasso
Copy link
Member Author

hirasso commented Nov 14, 2023

I guess the playwright shield status will appear when this is merged.

@hirasso
Copy link
Member Author

hirasso commented Nov 14, 2023

I decided to use "vitest" and "playwright" instead of "unit" and "e2e" since the vitest tests aren't all real unit tests. The line is blurry.

@daun
Copy link
Member

daun commented Nov 14, 2023

Makes sense! The reason I've used functional and unit was mostly to make it understandable for people who haven't used/heard of both tools, but that's probably not too many people anyway.

Copy link
Member

@daun daun left a comment

Choose a reason for hiding this comment

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

Nice work 🥯

@hirasso hirasso merged commit f405134 into master Nov 14, 2023
@hirasso hirasso deleted the feat/playwright-tests branch November 14, 2023 09:07
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.

2 participants