-
Notifications
You must be signed in to change notification settings - Fork 779
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
tests: start server on integration test #2664
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like we're intentionally adding a race condition, but I'm not sure how else to handle this. LGTM but we should keep an eye on this.
Nope, this is still timing out after 47 tests... I have no idea what is going on but I don't know how else to do this in a single command so we don't have to start the server beforehand |
If you like, we can pair on it next week. Should I set something up? |
IT WORKS! So this is the "best" way I've found to do this but it adds a lot of npm scripts.
|
Well done! Glad you figured it out! |
Closes issue: #2658
Reviewer checks
Required fields, to be filled out by PR reviewer(s)