chore: run website tests locally on forks in CI #7247
Conversation
|
Thanks for the PR, @rubiesonthesky! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. |
✅ Deploy Preview for typescript-eslint ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
This comment was marked as outdated.
This comment was marked as outdated.
7915c41 to
85328a9
Compare
…d test to check that website title is correct
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
packages/website/tests/index.spec.ts
Outdated
| @@ -21,7 +21,8 @@ test.describe('Website', () => { | |||
| } | |||
| errorMessages.push(`[${type}] ${text}`); | |||
| }); | |||
| await page.goto('/'); | |||
| await page.goto('/', { waitUntil: 'domcontentloaded' }); | |||
There was a problem hiding this comment.
This change may fix failing in main. At least it fixes local test run.
There was a problem hiding this comment.
@JoshuaKGoldberg if you have time, you could pick the changes from this file and create separate PR. I think netlify deploy + tests works for your PRs like it was main? My theory is that this would fix the problem in main but there is now way non-team member can really tests this before hand. :D
(Unless someone already fixed main)
Hmm, have you run |
That log is from CI. It May be that the package is not been built correctly in CI. I have disabled nx sensing anything from local to cloud so everything needs to happen in CI. I’m not familiar with nx and the setup tho. But to confirm, was there a reason why the test are run against netlify deploy? Like, the benefits are to test actual deploy and not needing to build the site in two places. But should the playwright tests work also in GitHub actions? :) |
|
cc @JamesHenry - IIRC it was that the Netlify deploys were much faster (when they exist), and GitHub-hosted deployments were rather flaky. |
|
I think this is now ready for review. The website test passes in PR. Some of those netlify tests are not getting notification that they have been done and instead they will just stay yellow. But that will need some maintainer looking into them later. |
There was a problem hiding this comment.
LGTM! I'm down to merge to main and see what happens... I'll send that non-fork PR now.
Will wait for @JamesHenry's approval since this touches Netlify & builds James was previously working on.
packages/website/tests/index.spec.ts
Outdated
| @@ -21,7 +21,8 @@ test.describe('Website', () => { | |||
| } | |||
| errorMessages.push(`[${type}] ${text}`); | |||
| }); | |||
| await page.goto('/'); | |||
| await page.goto('/', { waitUntil: 'domcontentloaded' }); | |||
Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
|
Created two new PRs from this one, so each will do just one thing
I will keep the same changes here to show that tests pass until those PRs have been merged to main. |
PR Checklist
Overview
This run website tests only if NETLIFY_TOKEN is available. The idea is, that if you change setting for when that token is available, then this step should automatically follow it. Let's see if this does work.