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

HttpWaitStrategy response body checking #441

Merged
merged 7 commits into from
Sep 19, 2017
Merged

HttpWaitStrategy response body checking #441

merged 7 commits into from
Sep 19, 2017

Conversation

barrycommins
Copy link
Contributor

@barrycommins barrycommins commented Aug 15, 2017

Hi,

I've added the ability to test the response body in HttpWaitStrategy as well as the response code.

This can be useful in cases where the status code isn't enough to tell if a service is fully initialised.

An example of this is a Spring Boot server making use of Eureka, where the /health endpoint might return 200, but the required downstream services might not have been registered yet.

In that case, you could parse the body, and traverse the json looking for the required values.

Example usage:

Wait.forHttp("/health")
    .forStatusCode(200)
    .forResponsePredicate(response -> JsonPath.read(response, 
        "$.discoveryComposite.discoveryClient.services").contains("some-service"));

or a simpler case, just checking an exact response:

Wait.forHttp("/")
    .forStatusCode(200)
    .forResponsePredicate(response->response.equals("some response"));

Any feedback or suggestions are welcome.

@kiview
Copy link
Member

kiview commented Aug 15, 2017

Great feature!
But I wonder, do we really need the three different methods? Isn't forResponsePredicate() enough? It might be a bit more clunky to use, but pretty straightforward using Java8 lambdas.

@barrycommins
Copy link
Contributor Author

@kiview ,

Good point, a lambda would achieve those other two functions very simply, and wouldn't clutter the api.
I'll remove them.

Copy link
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

I've proposed one additional minor change. Sorry for not looking into this PR sooner. I'll squash and merge the PR once this is changed.

@@ -114,6 +127,12 @@ protected void waitUntilReady() {
connection.getResponseCode()));
}

String responseBody = getResponseBody(connection);
Copy link
Member

Choose a reason for hiding this comment

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

We could move the parsing of the response body into the if clause. I would prefer this, since it's only needed if a responsePredicate is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, that makes sense. I've made the change now.

@kiview
Copy link
Member

kiview commented Sep 19, 2017

LGTM, I merge once Travis has finished.

@kiview kiview merged commit 66bd7c0 into testcontainers:master Sep 19, 2017
@barrycommins barrycommins deleted the waitstrategy-responsebody branch September 24, 2017 22:30
rnorth pushed a commit that referenced this pull request Apr 10, 2019
Bumps [amqp-client](https://github.com/rabbitmq/rabbitmq-java-client) from 5.6.0 to 5.7.0.
<details>
<summary>Release notes</summary>

*Sourced from [amqp-client's releases](https://github.com/rabbitmq/rabbitmq-java-client/releases).*

> ## 5.7.0
> # Changes between 5.6.0 and 5.7.0
> 
> This is a minor release with usability improvements and dependency upgrades. All users of the 5.x.x series are encouraged to upgrade to this version.
> 
> ## Improve logging for TLS connections
> 
> GitHub issue: [#441](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/441)
> 
> ## Don't panic when cancelling an unknown consumer
> 
> GitHub issue: [#525](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/525)
> 
> ## Bump dependencies
> 
> GitHub issue: [#607](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/607) 
> 
> ## 5.7.0.RC1
> # Changes between 5.6.0 and 5.7.0.RC1
> 
> This is a pre-release for 5.7.0, a maintenance release with usability improvements and dependency upgrades. All users of the 5.x.x series are encouraged to test this version.
> 
> ## Improve logging for TLS connections
> 
> GitHub issue: [#441](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/441)
> 
> ## Don't panic when cancelling an unknown consumer
> 
> GitHub issue: [#525](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/525)
> 
> ## Bump dependencies
> 
> GitHub issue: [#607](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/607) 
</details>
<details>
<summary>Changelog</summary>

*Sourced from [amqp-client's changelog](https://github.com/rabbitmq/rabbitmq-java-client/blob/v5.7.0/release-versions.txt).*

> RELEASE_VERSION="5.7.0"
> DEVELOPMENT_VERSION="5.8.0-SNAPSHOT"
</details>
<details>
<summary>Commits</summary>

- [`57aa724`](rabbitmq/rabbitmq-java-client@57aa724) [maven-release-plugin] prepare release v5.7.0
- [`1928ce8`](rabbitmq/rabbitmq-java-client@1928ce8) Set release version to 5.7.0
- [`9f2dffe`](rabbitmq/rabbitmq-java-client@9f2dffe) [maven-release-plugin] prepare for next development iteration
- [`e79ed4d`](rabbitmq/rabbitmq-java-client@e79ed4d) [maven-release-plugin] prepare release v5.7.0.RC1
- [`bb8f6ed`](rabbitmq/rabbitmq-java-client@bb8f6ed) Bump test dependencies
- [`953d255`](rabbitmq/rabbitmq-java-client@953d255) Bump dependencies
- [`e49f66d`](rabbitmq/rabbitmq-java-client@e49f66d) Add comment to explain decision tree
- [`365dd22`](rabbitmq/rabbitmq-java-client@365dd22) Add test for Channel#basicCancel with unknown consumer tag
- [`6081628`](rabbitmq/rabbitmq-java-client@6081628) Log warning when receiving basic.cancel for unknown consumer
- [`be57b26`](rabbitmq/rabbitmq-java-client@be57b26) Merge pull request [#548](https://github-redirect.dependabot.com/rabbitmq/rabbitmq-java-client/issues/548) from spring-operator/polish-urls-apache-license-5.x.x...
- Additional commits viewable in [compare view](rabbitmq/rabbitmq-java-client@v5.6.0...v5.7.0)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=com.rabbitmq:amqp-client&package-manager=gradle&previous-version=5.6.0&new-version=5.7.0)](https://dependabot.com/compatibility-score.html?dependency-name=com.rabbitmq:amqp-client&package-manager=gradle&previous-version=5.6.0&new-version=5.7.0)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
If all status checks pass Dependabot will automatically merge this pull request.

[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in the `.dependabot/config.yml` file in this repo:
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
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.

None yet

2 participants