-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Use java 21 to run tests #2174
Use java 21 to run tests #2174
Conversation
WalkthroughThis pull request primarily updates the Java version from 17 to 21 across the project. It also modifies the Docker image used in the CI pipeline and introduces changes to cache keys for dependency management. Additionally, new test methods are added to handle scenarios where the JaxRS client hangs if the response doesn't have a payload. Changes
TipsChat with CodeRabbit Bot (
|
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.
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.
Rate Limit ExceededYou have exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 29 minutes and 59 seconds before requesting another review. To re-trigger a review, convert this PR to a draft and then mark it as ready for review again. Alternatively, you can push a new commit to this PR. |
Rate Limit ExceededYou have exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 29 minutes and 59 seconds before requesting another review. To re-trigger a review, convert this PR to a draft and then mark it as ready for review again. Alternatively, you can push a new commit to this PR. |
Rate Limit ExceededYou have exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 29 minutes and 59 seconds before requesting another review. To re-trigger a review, convert this PR to a draft and then mark it as ready for review again. Alternatively, you can push a new commit to this PR. |
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.
Review Status
Actionable comments generated: 2
Files selected for processing (4)
- .circleci/config.yml (3 hunks)
- googlehttpclient/src/test/java/feign/googlehttpclient/GoogleHttpClientTest.java (1 hunks)
- jaxrs2/src/test/java/feign/jaxrs2/JAXRSClientTest.java (1 hunks)
- pom.xml (1 hunks)
Files skipped from review due to trivial changes (2)
- .circleci/config.yml
- pom.xml
@Override | ||
public void testVeryLongResponseNullLength() { | ||
assumeFalse("JaxRS client hang if the response doesn't have a payload", false); | ||
} |
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 new test method testVeryLongResponseNullLength()
is overriding a method from the parent class and includes an assumption that always fails. This will cause the test to be skipped in all scenarios. If this is intentional, please ensure that it's documented why this test is being skipped for the JaxRS client. If not, the assumption condition should be updated to reflect when the test should be skipped.
- assumeFalse("JaxRS client hang if the response doesn't have a payload", false);
+ // Update with correct condition
+ boolean condition = ...;
+ assumeFalse("JaxRS client hang if the response doesn't have a payload", condition);
@Override | ||
public void testVeryLongResponseNullLength() { | ||
assumeFalse("JaxRS client hang if the response doesn't have a payload", false); | ||
} |
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 new test method testVeryLongResponseNullLength()
is overriding a method from the parent class and making an assumption that always fails. This will cause the test to be skipped in all scenarios. If this is intentional, it's fine. However, if you intended to implement a real test here, you should replace the assumeFalse
statement with actual test logic.
- assumeFalse("JaxRS client hang if the response doesn't have a payload", false);
+ // Implement your test logic here
* Use java 21 to run tests * Skip tests that are not working on 21, must revert this at some point --------- Co-authored-by: Marvin Froeder <[email protected]>
* Use java 21 to run tests * Skip tests that are not working on 21, must revert this at some point --------- Co-authored-by: Marvin Froeder <[email protected]>
Summary by CodeRabbit
testVeryLongResponseNullLength()
in bothGoogleHttpClientTest
andJAXRSClientTestInterfaceWithJaxRsContract
to validate behavior when response doesn't have a payload.