8000 chore: website test: use waitUntil domcontentloaded by rubiesonthesky · Pull Request #7436 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

chore: website test: use waitUntil domcontentloaded #7436

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

chore: website test: use waitUntil domcontentloaded #7436

wants to merge 6 commits into from

Conversation

rubiesonthesky
Copy link
Contributor
@rubiesonthesky rubiesonthesky commented Aug 7, 2023 •
edited
Loading

PR Checklist

Overview

I don't know if the failing test is just flaky or what is going on. In this PR, all website tests are again passing. 🤷

@typescript-eslint
Copy link
Contributor

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.

@netlify
Copy link
netlify bot commented Aug 7, 2023 •
edited
Loading

✅ Deploy Preview for typescript-eslint ready!

Name Link
🔨 Latest commit 4c61df8
🔍 Latest deploy log https://app.netlify.com/sites/typescript-eslint/deploys/64dbeccae67ad10008a9c1a9
😎 Deploy Preview https://deploy-preview-7436--typescript-eslint.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.

@nx-cloud
Copy link
nx-cloud bot commented Aug 7, 2023 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 4c61df8. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 37 targets

Sent with 💌 from NxCloud.

@rubiesonthesky rubiesonthesky marked this pull request as ready for review August 7, 2023 13:55
@rubiesonthesky
Copy link
Contributor Author

@JoshuaKGoldberg It looks like this change makes test little bit less flaky? If the last run will return green then I think merging this will be positive change. Seems that most PRs are failing but not all.

it could be also that this is why the test were not run locally. But until more long term solution, it looks like this change could help. (If indeed the CI run that I triggered does not fail)

JoshuaKGoldberg
JoshuaKGoldberg requested changes Aug 10, 2023
View reviewed changes
Copy link
Member
@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

The waitUntil works for me, but let's revert retries?

@JoshuaKGoldberg JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Aug 10, 2023
@rubiesonthesky
Copy link
Contributor Author

Triggered 2nd run after changing retries to 0. (I want to have at least 2 passing runs before merge. Maybe 3 would be ideal but it’s easiest to trigger it after main has updated)

@JoshuaKGoldberg
Copy link
Member
JoshuaKGoldberg commented Aug 10, 2023 •
edited
Loading

Got a failure on a 3rd run :/ https://github.com/typescript-eslint/typescript-eslint/actions/runs/5821190200/job/15784867250

@rubiesonthesky
Copy link
Contributor Author

Okay. My next idea is that we need to same as with netlify, we need to ping the site until it’s actually running before doing tests.

Though, I think with this kind of test, flakiness is not so bad. Well, it’s better to have flaky test than no test.

Test report should show were the test flaky when retries was set to 1. I need to check that later when I’m on computer. It could be that it did not even help but the problem is somewhere else.

@rubiesonthesky rubiesonthesky marked this pull request as draft August 10, 2023 15:59
@rubiesonthesky
Copy link
Contributor Author

If someone else has ideas how to fix this or is quicker to try something out, I’m happy to close PR. I’m not sure how quickly I will have time to experiment with this. :)

I’ll keep the PR open for now to be reminder for myself and also show what was already tried. If there are no updates in few weeks, please do not hesitate to close this for repo upkeeping.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg requested changes

Assignees
No one assigned
Labels
awaiting response Issues waiting for a reply from the OP or another party
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Repo: One website test consistently failing in CI (page was closed)
2 participants
@rubiesonthesky @JoshuaKGoldberg
0