chore(#4884): upgrade qulice to 0.25.1 in integration tests#4910
Conversation
…ove test assertions
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughRefactors test classpath construction in JarIT, adds PMD suppression annotations across many integration tests, standardizes UTF-8 in test file writes, tweaks ProxyIT locals/assertions, and synchronizes qulice-maven-plugin version declarations across root and module POMs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Performance AnalysisAll benchmarks are within the acceptable range. No critical degradation detected (threshold is 100%). Please refer to the detailed report for more information. Click to see the detailed report
|
There was a problem hiding this comment.
Pull request overview
This PR updates the Maven build to use qulice-maven-plugin 0.25.1 and adjusts integration tests to comply with the stricter static-analysis rules triggered by the upgrade (notably PMD assertions/encodings and minor refactorings).
Changes:
- Bump
qulice-maven-pluginfrom0.25.0to0.25.1in the rootquliceprofile and remove redundant per-module version overrides. - Update integration tests to satisfy new Qulice/PMD checks (explicit UTF-8 in
getBytes, additional@SuppressWarnings). - Refactor a few tests (e.g., proxy and jar integration tests) to match updated linting expectations.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pom.xml |
Upgrades Qulice plugin version to 0.25.1 in the qulice profile. |
eo-runtime/pom.xml |
Removes module-level Qulice plugin version to rely on inherited configuration. |
eo-parser/pom.xml |
Removes module-level Qulice plugin version to rely on inherited configuration. |
eo-maven-plugin/pom.xml |
Removes module-level Qulice plugin version to rely on inherited configuration. |
eo-integration-tests/src/test/java/org/eolang/maven/ProxyIT.java |
Refactors proxy test assertions and addresses PMD rules; introduces Requisite typing. |
eo-integration-tests/src/test/java/org/eolang/maven/MjTranspileIT.java |
Adds PMD suppression; enforces explicit UTF-8 encoding. |
eo-integration-tests/src/test/java/org/eolang/maven/MjResolveIT.java |
Adds PMD suppression annotations. |
eo-integration-tests/src/test/java/org/eolang/maven/MjRegisterIT.java |
Adds PMD suppression; enforces explicit UTF-8 encoding; reformats suppression block. |
eo-integration-tests/src/test/java/org/eolang/maven/MjPrintIT.java |
Adds PMD suppression; enforces explicit UTF-8 encoding. |
eo-integration-tests/src/test/java/org/eolang/maven/MjParseIT.java |
Adds PMD suppression; enforces explicit UTF-8 encoding. |
eo-integration-tests/src/test/java/org/eolang/maven/MjLintIT.java |
Adds PMD suppression; enforces explicit UTF-8 encoding in writes. |
eo-integration-tests/src/test/java/org/eolang/maven/MjAssembleIT.java |
Adds PMD suppression annotations. |
eo-integration-tests/src/test/java/integration/XmirIT.java |
Adds PMD suppression annotation. |
eo-integration-tests/src/test/java/integration/SnippetIT.java |
Extends suppression list to include PMD assertion rule. |
eo-integration-tests/src/test/java/integration/ReadmeSnippetsIT.java |
Adds PMD suppression annotation. |
eo-integration-tests/src/test/java/integration/JarIT.java |
Adds PMD suppression and refactors runtime classpath construction. |
Comments suppressed due to low confidence (1)
eo-integration-tests/src/test/java/org/eolang/maven/ProxyIT.java:81
- The assertion message says the response body should contain "objectionary.com", but the matcher checks for "objectionary" and "sources". Please align the message with what is actually asserted (or adjust the matcher if the full domain is required).
MatcherAssert.assertThat(
"Response body should contain objectionary.com",
HttpClient.newBuilder()
.proxy(ProxySelector.of(new InetSocketAddress("localhost", this.port)))
.followRedirects(HttpClient.Redirect.NORMAL)
.build()
.send(
HttpRequest.newBuilder()
.uri(URI.create("https://objectionary.com/"))
.header("User-Agent", "test-client")
.GET()
.build(), HttpResponse.BodyHandlers.ofString()
).body(),
Matchers.allOf(
Matchers.containsString("objectionary"),
Matchers.containsString("sources")
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -72,15 +74,7 @@ void checksThatProxyIsWorking() throws IOException, InterruptedException { | |||
| .header("User-Agent", "test-client") | |||
| .GET() | |||
| .build(), HttpResponse.BodyHandlers.ofString() | |||
| ); | |||
| MatcherAssert.assertThat( | |||
| "Proxy should return 200 OK for objectionary.com", | |||
| resp.statusCode(), | |||
| Matchers.equalTo(HttpStatus.OK_200) | |||
| ); | |||
| MatcherAssert.assertThat( | |||
| "Response body should contain objectionary.com", | |||
| resp.body(), | |||
| ).body(), | |||
There was a problem hiding this comment.
checksThatProxyIsWorking() no longer verifies the HTTP status code, so the test could pass even if the proxy returns an error/redirect page that still contains the expected substrings. Consider keeping a separate assertion for statusCode() (e.g., equals 200) in addition to the body-content assertion.
| Paths.get(System.getProperty("user.home")).resolve(".m2") | ||
| .resolve("repository") | ||
| .resolve("org/eolang/eo-runtime") | ||
| .resolve( | ||
| System.getProperty("eo.version", Manifests.read("EO-Version")) | ||
| ) | ||
| .resolve( | ||
| String.format( | ||
| "eo-runtime-%s.jar", | ||
| System.getProperty("eo.version", Manifests.read("EO-Version")) | ||
| ) | ||
| ) | ||
| .toString() |
There was a problem hiding this comment.
compile() now recomputes the EO runtime version via System.getProperty("eo.version", Manifests.read("EO-Version")) multiple times when building the runtime JAR path. Storing the resolved version in a local variable once would improve readability and avoid repeated manifest reads/property lookups, ensuring the same version is used consistently throughout the path construction.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
eo-integration-tests/src/test/java/integration/JarIT.java (1)
251-264: Consider extracting the version to a local variable.
System.getProperty("eo.version", Manifests.read("EO-Version"))is evaluated twice (lines 255 and 260). While functionally correct, extracting it to a local variable would improve readability and follow DRY.♻️ Suggested refactor
+ final String version = System.getProperty("eo.version", Manifests.read("EO-Version")); return String.join( File.pathSeparator, "test-0.0.0.jar", Paths.get(System.getProperty("user.home")).resolve(".m2") - .resolve("repository") - .resolve("org/eolang/eo-runtime") - .resolve( - System.getProperty("eo.version", Manifests.read("EO-Version")) - ) - .resolve( - String.format( - "eo-runtime-%s.jar", - System.getProperty("eo.version", Manifests.read("EO-Version")) - ) - ) - .toString() + .resolve("repository") + .resolve("org/eolang/eo-runtime") + .resolve(version) + .resolve(String.format("eo-runtime-%s.jar", version)) + .toString() );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@eo-integration-tests/src/test/java/integration/JarIT.java` around lines 251 - 264, Extract the repeated System.getProperty("eo.version", Manifests.read("EO-Version")) into a local variable (e.g., eoVersion) at the start of the method in JarIT, then replace both occurrences in the Paths.get(...) chain and the String.format("eo-runtime-%s.jar", ...) call with that variable; this removes the duplicated call, improves readability, and keeps behavior identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@eo-integration-tests/src/test/java/integration/JarIT.java`:
- Around line 251-264: Extract the repeated System.getProperty("eo.version",
Manifests.read("EO-Version")) into a local variable (e.g., eoVersion) at the
start of the method in JarIT, then replace both occurrences in the
Paths.get(...) chain and the String.format("eo-runtime-%s.jar", ...) call with
that variable; this removes the duplicated call, improves readability, and keeps
behavior identical.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
eo-integration-tests/src/test/java/integration/JarIT.javaeo-integration-tests/src/test/java/integration/ReadmeSnippetsIT.javaeo-integration-tests/src/test/java/integration/SnippetIT.javaeo-integration-tests/src/test/java/integration/XmirIT.javaeo-integration-tests/src/test/java/org/eolang/maven/MjAssembleIT.javaeo-integration-tests/src/test/java/org/eolang/maven/MjLintIT.javaeo-integration-tests/src/test/java/org/eolang/maven/MjParseIT.javaeo-integration-tests/src/test/java/org/eolang/maven/MjPrintIT.javaeo-integration-tests/src/test/java/org/eolang/maven/MjRegisterIT.javaeo-integration-tests/src/test/java/org/eolang/maven/MjResolveIT.javaeo-integration-tests/src/test/java/org/eolang/maven/MjTranspileIT.javaeo-integration-tests/src/test/java/org/eolang/maven/ProxyIT.javaeo-maven-plugin/pom.xmleo-parser/pom.xmleo-runtime/pom.xmlpom.xml
💤 Files with no reviewable changes (3)
- eo-maven-plugin/pom.xml
- eo-runtime/pom.xml
- eo-parser/pom.xml
|
@yegor256 could you have a look, please? |
|
@volodya-lombrozo Great work on your contribution! 🎉 You've earned +12 points (+16 base, -4 for code volume of 176 lines). Quality over speed keeps us moving forward! Your total score is now +397 — check your Zerocracy account for updates. |
This PR upgrades
quliceto version 0.25.1 in integration tests.Fixes #4884
Summary by CodeRabbit