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

Listen for development server on different port for tests #622

Merged
merged 2 commits into from
Mar 28, 2018

Conversation

NickColley
Copy link
Contributor

@NickColley NickColley commented Mar 27, 2018

Required some refactoring of how we listen since the tests were not actually,
adding a new listener.

Also fixes issues with tests not closing properly by allowing tests to turn off
nunjucks watch mode.

Used https://github.com/mafintosh/why-is-node-running to identify and fix this issue.

Use https://github.com/alphagov/govuk-frontend/pull/622/files?w=1 to see changes without whitespace updates, since I've indented a lot of code.

Fixes #612

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 27, 2018 16:19 Inactive
@NickColley NickColley force-pushed the fix-tests-colliding-with-dev-server branch from dabe6f8 to 864c3da Compare March 27, 2018 17:14
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 27, 2018 17:14 Inactive
@NickColley NickColley force-pushed the fix-tests-colliding-with-dev-server branch from 864c3da to c31ea93 Compare March 27, 2018 17:16
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 27, 2018 17:16 Inactive
@kr8n3r
Copy link

kr8n3r commented Mar 27, 2018

do you also need to change the heroku script in package.json?

@NickColley NickColley force-pushed the fix-tests-colliding-with-dev-server branch from c31ea93 to d294ee8 Compare March 27, 2018 17:22
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 27, 2018 17:22 Inactive
@NickColley NickColley force-pushed the fix-tests-colliding-with-dev-server branch from d294ee8 to f214d4d Compare March 27, 2018 17:28
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 27, 2018 17:29 Inactive
@NickColley NickColley force-pushed the fix-tests-colliding-with-dev-server branch from f214d4d to 02bfb3d Compare March 27, 2018 17:42
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 27, 2018 17:42 Inactive
@NickColley NickColley force-pushed the fix-tests-colliding-with-dev-server branch from 02bfb3d to d62c8e3 Compare March 27, 2018 17:44
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 27, 2018 17:44 Inactive
@kr8n3r
Copy link

kr8n3r commented Mar 28, 2018

could we have http://localhost:8888/ some where central, as we're repeating it in multiple places?

@NickColley
Copy link
Contributor Author

NickColley commented Mar 28, 2018

I don't like using globals in Node.js projects, so tried that but it felt over-the-top, this repeats it but requires less thinking to understand the code.

Happy to put it back in though

@kr8n3r
Copy link

kr8n3r commented Mar 28, 2018

don't forget the changelog as well

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 28, 2018 09:57 Inactive
@NickColley NickColley force-pushed the fix-tests-colliding-with-dev-server branch from 80140af to 541f483 Compare March 28, 2018 10:03
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 28, 2018 10:04 Inactive
@NickColley NickColley force-pushed the fix-tests-colliding-with-dev-server branch from 541f483 to e41e605 Compare March 28, 2018 10:05
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-622 March 28, 2018 10:06 Inactive
Required some refactoring of how we listen since the tests were not actually,
adding a new listener.

Fixes issues with tests not closing properly by allowing tests to turn off
nunjucks watch mode.

Used https://github.com/mafintosh/why-is-node-running to identify and fix this issue.
Copy link

@kr8n3r kr8n3r left a comment

Choose a reason for hiding this comment

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

💯

@NickColley NickColley merged commit a4860b9 into master Mar 28, 2018
@NickColley NickColley deleted the fix-tests-colliding-with-dev-server branch March 28, 2018 10:34
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.

Tests run on the same port as the review application.
3 participants