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

Terminate dev test processes with prejudice on exit #620

Closed
wants to merge 1 commit into from

Conversation

GregBrimble
Copy link
Contributor

Fixes #397 (and #618)

Kills the dev processes aggressively when jest is finished its tests of the example repos.

Seems a handy utility. Maybe we use it elsewhere as well?

@threepointone
Copy link
Contributor

Is there a solution where we actually clean up correctly instead of doing this? What is the source of the hanging processes/ports?

@threepointone
Copy link
Contributor

might be related #543

@GregBrimble
Copy link
Contributor Author

#543 is what I was hinting at where we could use this elsewhere. Inside the pages dev command, we spin up a couple of processes, and we certainly do try to clean them up on exit. But this utility might be doing a better job than I can. So maybe we swap out my logic in that command for this as well?

@threepointone
Copy link
Contributor

I really don't want us to install a non trivial dependency for something that seems like we should be getting right. Can we investigate why we're struggling to close this process at all, if you're saying we already have code to do so?

@changeset-bot
Copy link

changeset-bot bot commented Mar 16, 2022

⚠️ No Changeset found

Latest commit: 03e7aea

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Why did the CI not run?

@@ -29,6 +29,7 @@
"jest": "^27.5.1",
"npm-run-all": "^4.1.5",
"prettier": "^2.5.1",
"terminate": "2.5.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this needs to be here, right?

@threepointone
Copy link
Contributor

GitHub actions (and other GitHub services) was having problems yesterday

@threepointone
Copy link
Contributor

Could we resolve this soon-ish please? It's starting to affect builds in CI now too

@petebacondarwin petebacondarwin added the pages Relating to Pages label Mar 22, 2022
@GregBrimble
Copy link
Contributor Author

New PR up in #670

@GregBrimble GregBrimble deleted the terminate-dev-test-processes branch March 22, 2022 14:10
mrbbot added a commit that referenced this pull request Oct 31, 2023
mrbbot added a commit that referenced this pull request Nov 1, 2023
mrbbot added a commit that referenced this pull request Nov 1, 2023
mrbbot added a commit that referenced this pull request Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Maintenance task pages Relating to Pages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chore: tests in example-remix-app leaves a hanging esbuild process
3 participants