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

Fix #2714 - Start demo server before running integration tests #3298

Merged

Conversation

bmf-ribeiro
Copy link
Contributor

Fixes #2714

  • Start demo server before running integration tests
  • Use port 3001 instead of 3000 to not conflict with the demo

 - Use port 3001 instead of  300 to not conflict with the demo
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

This is great!

We can also remove the hacky server starts in the linux and mac integration tests jobs, reducing CI time by ~10 seconds:

- script: |
yarn start &
sleep 10
displayName: 'Start test server'

@Tyriar Tyriar added this to the 4.12.0 milestone Apr 6, 2021
@bmf-ribeiro
Copy link
Contributor Author

bmf-ribeiro commented Apr 6, 2021

Okay, that makes sense, should the integration tests start running on windows as well, then?

  • Looks like I need to await the server start, because the first tests did not run this time...

@Tyriar
Copy link
Member

Tyriar commented Apr 6, 2021

It would be good to run windows too, I forgot why they were disabled though. Can try enable in a separate pr and see what happens?

@bmf-ribeiro
Copy link
Contributor Author

This is ready to review.

Also I opened #3299 the tests ssem to be running okay on windows.

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

🎉

@Tyriar Tyriar merged commit 6370a93 into xtermjs:master Apr 6, 2021
@bmf-ribeiro bmf-ribeiro deleted the feat/2714-start-server-on-integration-tests branch April 6, 2021 20:00
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.

Have integration tests run their own server on a port other than 3000
2 participants