Adding listen address option so that IPv6 and loopback interfaces can be used. #2479

grifball · 2016-12-03T22:00:18Z

reference: #2477

…work correctly with it reference: karma-runner#2477

googlebot · 2016-12-03T22:00:20Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

grifball · 2016-12-03T22:04:42Z

I signed the cla. Not exactly sure how this works. Is that all I have to do?

googlebot · 2016-12-03T22:04:43Z

CLAs look good, thanks!

maksimr · 2016-12-03T22:17:53Z

LGTM

cc @dignifiedquire

wesleycho · 2016-12-06T05:46:19Z

test/unit/server.spec.js

+        if (typeof (arg2) === 'function') {
+          callback = arg2
+        } else
+        if (typeof (arg3) === 'function') {


This if can probably be moved up a line so that it reads else if (...) {, which is more standard

maksimr · 2016-12-06T08:12:38Z

@grifball could you squash commits in to one

Thanks

dignifiedquire

This should influence the value of config.hostname, because now that this can be changed to any value, config.hostname going to localhost can fail.

grifball · 2016-12-06T21:55:20Z

I started writing something that automatically tries to set hostname when listenAddress is set but hostname isn't, but it seems out of scope. Figuring out which IP to use for hostname based on the value of listenAddress is really difficult, and can even be impossible in more complex interface setups.
I think that a better solution would be to give a warning if listenAddress is set and hostname isn't. Something like: "ListenAddress is set but hostname isn't, if your browsers fail to connect, consider setting the hostname option."

grifball · 2016-12-06T23:51:19Z

Let me know what you think of this solution. I'm not sure how to change the label on this PR.
I'll squash the commits once the code looks good.

dignifiedquire · 2016-12-07T10:32:31Z

lib/config.js

+    testConfig.set(cliOptions)
+  }
+  if (testConfig.hostname == null && testConfig.listenAddress != null) {
+    log.warn('ListenAddress is set but hostname isn\'t. If your browsers fail ' +


It would be nice to print the current values of both the variables here.

dignifiedquire · 2016-12-07T10:34:20Z

lib/config.js

+  testConfig.hostname = null
+  testConfig.listenAddress = null
+  if (configModule != null) {
+    configModule(testConfig)


Let's not do this, as this means we are evaluating the code from the user twice. Best case this slows things down, worst case it has unexpected side effects. You can add the check probably around the original invocition of the configModule

dignifiedquire · 2016-12-07T10:34:46Z

Thank you, yes I think it's fine to just warn the user about this. Left some comments about the implementation.

grifball · 2016-12-07T19:25:28Z

Alright, I implemented those changes. Let me know if anything else needs changing.

wesleycho · 2016-12-07T19:28:21Z

Do we want weak equality (== & !=) checks here? Seems like a better way would be to check specifically if it is a non-trivial string.

grifball · 2016-12-07T20:03:59Z

This should be a safe enough check. Unless they're setting these values to null and expecting the server to work correctly, which explicit string checks wouldn't solve anyway.
If they're setting these values to non-string types, I assume there's other checks in the config file for this, but it's out of scope in this PR anyway. The added code won't change how we handle config type checking.

dignifiedquire

This is looking great, but I just realised there are not really tests covering this. Could you add some please?

grifball · 2016-12-10T22:17:31Z

I added a simple test to make sure that listen address was called with the configured listenAddress. That should cover all the functionality for this PR.

dignifiedquire · 2016-12-11T12:20:59Z

Thank you

This docs update was missing in the karma-runner#2479 that added option for configurable listening address. Thanks!

This docs update was missing in the #2479 that added option for configurable listening address.

# [5.0.0](v4.4.1...v5.0.0) (2020-04-09) ### Bug Fixes * install semantic-release as a regular dev dependency ([#3455](#3455)) ([1eaf35e](1eaf35e)) * **ci:** echo travis env that gates release after_success ([#3446](#3446)) ([b8b2ed8](b8b2ed8)) * **ci:** poll every 10s to avoid rate limit. ([#3388](#3388)) ([91e7e00](91e7e00)) * **middleware/runner:** handle file list rejections ([#3400](#3400)) ([80febfb](80febfb)), closes [#3396](#3396) [#3396](#3396) * **server:** cleanup import of the removed method ([#3439](#3439)) ([cb1bcbf](cb1bcbf)) * **server:** createPreprocessor was removed ([#3435](#3435)) ([5c334f5](5c334f5)) * **server:** detection new MS Edge Chromium ([#3440](#3440)) ([7166ce2](7166ce2)) * **server:** replace optimist on yargs lib ([#3451](#3451)) ([ec1e69a](ec1e69a)), closes [#2473](#2473) * **server:** Report original error message ([#3415](#3415)) ([79ee331](79ee331)), closes [#3414](#3414) ### Code Refactoring * use native Promise instead of Bluebird ([#3436](#3436)) ([33a069f](33a069f)), closes [/github.com//pull/3060#discussion_r284797390](https://github.com//github.com/karma-runner/karma/pull/3060/issues/discussion_r284797390) ### Continuous Integration * drop node 8, adopt node 12 ([#3430](#3430)) ([a673aa8](a673aa8)) ### Features * **docs:** document `DEFAULT_LISTEN_ADDR` constant ([#3443](#3443)) ([057d527](057d527)), closes [#2479](#2479) * **karma-server:** added log to the server.js for uncaught exception ([#3399](#3399)) ([adc6a66](adc6a66)) * **preprocessor:** obey Pattern.isBinary when set ([#3422](#3422)) ([708ae13](708ae13)), closes [#3405](#3405) ### BREAKING CHANGES * Karma plugins which rely on the fact that Karma uses Bluebird promises may break as Bluebird-specific API is no longer available on Promises returned by the Karma core * **server:** Deprecated createPreprocessor removed, karma-browserify < 7 version doesn't work * no more testing on node 8.

This docs update was missing in the karma-runner#2479 that added option for configurable listening address.

# [5.0.0](karma-runner/karma@v4.4.1...v5.0.0) (2020-04-09) ### Bug Fixes * install semantic-release as a regular dev dependency ([karma-runner#3455](karma-runner#3455)) ([1eaf35e](karma-runner@1eaf35e)) * **ci:** echo travis env that gates release after_success ([karma-runner#3446](karma-runner#3446)) ([b8b2ed8](karma-runner@b8b2ed8)) * **ci:** poll every 10s to avoid rate limit. ([karma-runner#3388](karma-runner#3388)) ([91e7e00](karma-runner@91e7e00)) * **middleware/runner:** handle file list rejections ([karma-runner#3400](karma-runner#3400)) ([80febfb](karma-runner@80febfb)), closes [karma-runner#3396](karma-runner#3396) [karma-runner#3396](karma-runner#3396) * **server:** cleanup import of the removed method ([karma-runner#3439](karma-runner#3439)) ([cb1bcbf](karma-runner@cb1bcbf)) * **server:** createPreprocessor was removed ([karma-runner#3435](karma-runner#3435)) ([5c334f5](karma-runner@5c334f5)) * **server:** detection new MS Edge Chromium ([karma-runner#3440](karma-runner#3440)) ([7166ce2](karma-runner@7166ce2)) * **server:** replace optimist on yargs lib ([karma-runner#3451](karma-runner#3451)) ([ec1e69a](karma-runner@ec1e69a)), closes [karma-runner#2473](karma-runner#2473) * **server:** Report original error message ([karma-runner#3415](karma-runner#3415)) ([79ee331](karma-runner@79ee331)), closes [karma-runner#3414](karma-runner#3414) ### Code Refactoring * use native Promise instead of Bluebird ([karma-runner#3436](karma-runner#3436)) ([33a069f](karma-runner@33a069f)), closes [/github.com/karma-runner/pull/3060#discussion_r284797390](https://github.com//github.com/karma-runner/karma/pull/3060/issues/discussion_r284797390) ### Continuous Integration * drop node 8, adopt node 12 ([karma-runner#3430](karma-runner#3430)) ([a673aa8](karma-runner@a673aa8)) ### Features * **docs:** document `DEFAULT_LISTEN_ADDR` constant ([karma-runner#3443](karma-runner#3443)) ([057d527](karma-runner@057d527)), closes [karma-runner#2479](karma-runner#2479) * **karma-server:** added log to the server.js for uncaught exception ([karma-runner#3399](karma-runner#3399)) ([adc6a66](karma-runner@adc6a66)) * **preprocessor:** obey Pattern.isBinary when set ([karma-runner#3422](karma-runner#3422)) ([708ae13](karma-runner@708ae13)), closes [karma-runner#3405](karma-runner#3405) ### BREAKING CHANGES * Karma plugins which rely on the fact that Karma uses Bluebird promises may break as Bluebird-specific API is no longer available on Promises returned by the Karma core * **server:** Deprecated createPreprocessor removed, karma-browserify < 7 version doesn't work * no more testing on node 8.

feat(server): Added 'listenAddress' option and updated unit tests to …

4e2d17a

…work correctly with it reference: karma-runner#2477

maksimr added the needs: review label Dec 3, 2016

wesleycho reviewed Dec 6, 2016

View reviewed changes

fix(test): making the if look better and removing uneccessary parens

c4fc0a7

dignifiedquire requested changes Dec 6, 2016

View reviewed changes

dignifiedquire added needs: feedback-from-author and removed needs: review labels Dec 6, 2016

grifball added 2 commits December 6, 2016 21:56

fix(server): warn the user if listenAddress is set but hostname isn't

75e4306

fix(config): some typos

5e29fd9

maksimr added needs: review and removed needs: feedback-from-author labels Dec 7, 2016

dignifiedquire reviewed Dec 7, 2016

View reviewed changes

dignifiedquire added needs: feedback-from-author and removed needs: review labels Dec 7, 2016

fix(config): better checking for overwritten options in config

d8eadba

fix(config): safer checks for unset options

a0bf79e

dignifiedquire requested changes Dec 9, 2016

View reviewed changes

feat(test): added a unit test for listenAddress

d931ecf

dignifiedquire approved these changes Dec 11, 2016

View reviewed changes

dignifiedquire merged commit 8e5bee6 into karma-runner:master Dec 11, 2016

dignifiedquire removed the needs: feedback-from-author label Dec 11, 2016

peterblazejewicz added a commit to peterblazejewicz/karma that referenced this pull request Mar 8, 2020

feat(docs): document DEFAULT_LISTEN_ADDR constant

1e8b2fe

This docs update was missing in the karma-runner#2479 that added option for configurable listening address. Thanks!

peterblazejewicz mentioned this pull request Mar 8, 2020

feat(docs): document DEFAULT_LISTEN_ADDR constant #3443

Merged

johnjbarton pushed a commit that referenced this pull request Mar 16, 2020

feat(docs): document DEFAULT_LISTEN_ADDR constant (#3443)

057d527

This docs update was missing in the #2479 that added option for configurable listening address.

karronoli mentioned this pull request Aug 16, 2022

Bump karma from ~1.2.0 to ~6.4 in /redpen-server karronoli/redpen#10

Merged

anthony-redFox pushed a commit to anthony-redFox/karma that referenced this pull request May 16, 2023

feat(docs): document DEFAULT_LISTEN_ADDR constant (karma-runner#3443)

50a720d

This docs update was missing in the karma-runner#2479 that added option for configurable listening address.

This was referenced Jan 1, 2024

[Snyk] Fix for 1 vulnerabilities Omrisnyk/npm-lockfiles#136

Open

[Snyk] Security upgrade karma from 2.0.3 to 5.0.8 Omrisnyk/npm-lockfiles#137

Open

Omrisnyk mentioned this pull request Feb 2, 2024

[Snyk] Security upgrade karma from 2.0.3 to 5.0.8 Omrisnyk/npm-lockfiles#139

Open

Omrisnyk mentioned this pull request Feb 11, 2024

[Snyk] Fix for 1 vulnerabilities Omrisnyk/npm-lockfiles#143

Open

Omrisnyk mentioned this pull request Mar 15, 2024

[Snyk] Fix for 1 vulnerabilities Omrisnyk/npm-lockfiles#148

Open

Omrisnyk mentioned this pull request May 28, 2024

[Snyk] Fix for 1 vulnerabilities Omrisnyk/npm-lockfiles#167

Open

Omrisnyk mentioned this pull request Jun 19, 2024

[Snyk] Fix for 1 vulnerabilities Omrisnyk/npm-lockfiles#169

Open

Omrisnyk mentioned this pull request Jul 2, 2024

[Snyk] Fix for 1 vulnerabilities Omrisnyk/npm-lockfiles#170

Open

Omrisnyk mentioned this pull request Oct 29, 2024

[Snyk] Fix for 2 vulnerabilities Omrisnyk/npm-lockfiles#201

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding listen address option so that IPv6 and loopback interfaces can be used. #2479

Adding listen address option so that IPv6 and loopback interfaces can be used. #2479

grifball commented Dec 3, 2016

googlebot commented Dec 3, 2016

grifball commented Dec 3, 2016

googlebot commented Dec 3, 2016

maksimr commented Dec 3, 2016 •

edited

Loading

wesleycho Dec 6, 2016

maksimr commented Dec 6, 2016

dignifiedquire left a comment

grifball commented Dec 6, 2016 •

edited

Loading

grifball commented Dec 6, 2016

dignifiedquire Dec 7, 2016

dignifiedquire Dec 7, 2016

dignifiedquire commented Dec 7, 2016

grifball commented Dec 7, 2016

wesleycho commented Dec 7, 2016

grifball commented Dec 7, 2016

dignifiedquire left a comment

grifball commented Dec 10, 2016

dignifiedquire commented Dec 11, 2016

Adding listen address option so that IPv6 and loopback interfaces can be used. #2479

Adding listen address option so that IPv6 and loopback interfaces can be used. #2479

Conversation

grifball commented Dec 3, 2016

googlebot commented Dec 3, 2016

grifball commented Dec 3, 2016

googlebot commented Dec 3, 2016

maksimr commented Dec 3, 2016 • edited Loading

wesleycho Dec 6, 2016

Choose a reason for hiding this comment

maksimr commented Dec 6, 2016

dignifiedquire left a comment

Choose a reason for hiding this comment

grifball commented Dec 6, 2016 • edited Loading

grifball commented Dec 6, 2016

dignifiedquire Dec 7, 2016

Choose a reason for hiding this comment

dignifiedquire Dec 7, 2016

Choose a reason for hiding this comment

dignifiedquire commented Dec 7, 2016

grifball commented Dec 7, 2016

wesleycho commented Dec 7, 2016

grifball commented Dec 7, 2016

dignifiedquire left a comment

Choose a reason for hiding this comment

grifball commented Dec 10, 2016

dignifiedquire commented Dec 11, 2016

maksimr commented Dec 3, 2016 •

edited

Loading

grifball commented Dec 6, 2016 •

edited

Loading