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

#431 CustomTestKit default config scans base config for listeners... #432

Merged
merged 1 commit into from
Mar 23, 2017
Merged

Conversation

akara
Copy link
Contributor

@akara akara commented Mar 22, 2017

... and assign port to 0 for all by default to avoid any listener port conflict.

Thanks for your pull request. Please review the following guidelines.

  • Title includes issue id.
  • Description of the change added.
  • Commits are squashed.
  • Tests added.
  • Documentation added/updated.
  • Also please review CONTRIBUTING.md.

|}
""".stripMargin + portOverrides
)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: Since the test already tests "default-listener" which is the result of the listener scan, this change is already tested with the current tests. No new tests needed.

docs/testing.md Outdated
@@ -108,7 +108,7 @@ Please note, `CustomTestKit` allows passing `config` and `resources` together as

#### Port binding in tests

Starting services requires port binding. To prevent port conflicts, we should let the system pick a port by setting listeners' `bind-port` to 0, e.g., `default-listener.bind-port = 0` (this is what `CustomTestKit` sets by default). `squbs-testkit` comes with a `trait` named `PortGetter` that allows retrieving the port picked by the system. `CustomTestKit` comes with `PortGetter` already mixed in, so you can use `port` value.
Starting services requires port binding. To prevent port conflicts, we should let the system pick a port by setting listeners' `bind-port` to 0, e.g., `default-listener.bind-port = 0` (`CustomTestKit` sets `bind-port = 0` for all listeners with the default configuration). `squbs-testkit` comes with a `trait` named `PortGetter` that allows retrieving the port picked by the system. `CustomTestKit` comes with `PortGetter` already mixed in, so you can use `port` value.
Copy link
Collaborator

Choose a reason for hiding this comment

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

(CustomTestKit sets bind-port = 0 for all listeners with the default configuration).

This sentence can be made little more clear. It sounds like "with the default configuration" is for "all listeners" while you are referring to CustomTestKit's default configuration. How about the following?

CustomTestKit, if used with default configuration, sets bind-port = 0 for all listeners.

@anilgursel
Copy link
Collaborator

Minor doc comment, otherwise LGTM

… assign port to 0 for all by default to avoid any listener port conflict.
@anilgursel anilgursel merged commit 2759c68 into paypal:master Mar 23, 2017
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.

2 participants