-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Bump rest-assured.version from 5.3.0 to 5.3.2 #36080
Conversation
This comment has been minimized.
This comment has been minimized.
Issue is:
I suppose we would have to enforce hamcrest version somewhere. |
352928f
to
dcf9832
Compare
I enforced the hamcrest version but I'm really not sure it's a good idea to enforce the RESTAssured, Awaitility and hamcrest version in our BOM. I wonder if we should move all these to the build-parent instead. Obviously, that would be a breaking change. |
This comment has been minimized.
This comment has been minimized.
Bumps `rest-assured.version` from 5.3.0 to 5.3.2. Updates `io.rest-assured:rest-assured` from 5.3.0 to 5.3.2 - [Changelog](https://github.com/rest-assured/rest-assured/blob/master/changelog.txt) - [Commits](https://github.com/rest-assured/rest-assured/commits) Updates `io.rest-assured:json-schema-validator` from 5.3.0 to 5.3.2 Updates `io.rest-assured:kotlin-extensions` from 5.3.0 to 5.3.2 --- updated-dependencies: - dependency-name: io.rest-assured:rest-assured dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: io.rest-assured:json-schema-validator dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: io.rest-assured:kotlin-extensions dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Both REST Assured and Awaitility depend on hamcrest. I'm not sure it's a good idea to have these three in the BOM really and it might be a good idea to move them out of the BOM and let users define the versions. We will see how it goes.
I had a closer look at the test following the new failure and AFAICT the test assertion looks incorrect, we should arrive on landing page first.
dcf9832
to
3563cda
Compare
@stuartwdouglas any chance you could have a look at this commit: 3563cda ? My understanding is that the test assertion was wrong from the beginning and somehow REST Assured wasn't checking things properly. Not sure if you remember as it has been a while. @cescoffier could you have a look too to make sure I didn't go crazy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assertion change is a little annoying. We need to investigate.
@@ -84,7 +84,7 @@ public void testFormBasedAuthSuccess() { | |||
.then() | |||
.assertThat() | |||
.statusCode(302) | |||
.header("location", containsString("/admin")) | |||
.header("location", containsString("/landing")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Weird...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I didn't change it lightly. From what I saw in the test, this is actually the right behavior. Given it's triggered by a REST Assured update, I suspect it was a bug in REST Assured.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our cookie uses redirect.setMaxAge(0)
. I'm wondering how Restassured handles this case (if it does).
I guess it is a change in the cookie behaviour. I think this is ok, it would be good to understand exactly what changed though. My guess is that with the old version the cookie that stores the redirect back location is still being sent, but with the new version it is not. I am guessing that they did not handle cookies with max-age=0 correctly, and now they have fixed it. |
Yeah, I saw a couple of commits related to that. This one by @FroMage might be related: rest-assured/rest-assured@a68bcc5 |
That is probably it. |
Failing Jobs - Building 3563cda
Full information is available in the Build summary check run. Failures⚙️ Maven Tests - JDK 11 Windows #📦 integration-tests/maven✖
✖
✖
✖
✖
✖
|
Bumps
rest-assured.version
from 5.3.0 to 5.3.2.Updates
io.rest-assured:rest-assured
from 5.3.0 to 5.3.2Changelog
Sourced from io.rest-assured:rest-assured's changelog.
Commits
Updates
io.rest-assured:json-schema-validator
from 5.3.0 to 5.3.2Updates
io.rest-assured:kotlin-extensions
from 5.3.0 to 5.3.2You can trigger a rebase of this PR by commenting
@dependabot rebase
.Dependabot commands and options
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 close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions
will show all of the ignore conditions of the specified dependency@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor 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)