Conversation
📝 WalkthroughWalkthroughTest expectations updated to account for Windows-style absolute paths when combining file URLs with relative path components, ensuring correct path resolution behavior across platforms. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/src/test/java/com/predic8/membrane/core/resolver/ResolverMapCombineTest.java`:
- Around line 66-67: The tests in ResolverMapCombineTest use hardcoded
"file:/C:/" URIs which fail on non-C drives; update the assertions that call
combine(...) and wl(...) to build expected file URIs dynamically using the
existing platform-aware helper/variable (e.g., the current variable used
elsewhere in the test) or by converting a java.io.File to a file URI (via
File#getAbsoluteFile or File#toURI) so the expected string matches the runtime
drive; adjust assertions referencing combine("file:/chi/elm","gnat") and other
hardcoded "file:/C:..." occurrences to use that computed URI instead.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/src/test/java/com/predic8/membrane/core/resolver/ResolverMapCombineTest.java
| assertEquals(wl("file:/C:/chi/gnat", | ||
| "file:/chi/gnat"), combine("file:/chi/elm","gnat")); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ResolverMap.java" -type fRepository: membrane/api-gateway
Length of output: 136
🏁 Script executed:
rg -n "public.*combine" --type=java -A 30 | head -100Repository: membrane/api-gateway
Length of output: 5255
🏁 Script executed:
rg -n "combine" core/src/main/java/com/predic8/membrane/core/resolver/ResolverMap.java -A 15 | head -80Repository: membrane/api-gateway
Length of output: 2952
🏁 Script executed:
find . -name "URIUtil.java" -o -name "FileUtil.java" | head -5Repository: membrane/api-gateway
Length of output: 194
🏁 Script executed:
rg -n "pathFromFileURI|convertPath2FilePathString" --type=java -B 2 -A 10 | head -100Repository: membrane/api-gateway
Length of output: 10905
🏁 Script executed:
# Check URIUtil class
rg -n "class URIUtil" --type=java -A 40Repository: membrane/api-gateway
Length of output: 7905
🏁 Script executed:
rg -n "getDirectoryPart|public.*resolve" core/src/main/java/com/predic8/membrane/core/util/FileUtil.java -A 8Repository: membrane/api-gateway
Length of output: 617
🏁 Script executed:
# Get full FileUtil class to understand resolve method
cat -n core/src/main/java/com/predic8/membrane/core/util/FileUtil.java | head -150Repository: membrane/api-gateway
Length of output: 4319
🏁 Script executed:
# Look at the full test file to see current variable and other file URI tests
cat -n core/src/test/java/com/predic8/membrane/core/resolver/ResolverMapCombineTest.java | head -150Repository: membrane/api-gateway
Length of output: 5607
🏁 Script executed:
# Check the `@BeforeAll` setup
rg -n "@BeforeAll|current" core/src/test/java/com/predic8/membrane/core/resolver/ResolverMapCombineTest.java -B 2 -A 3Repository: membrane/api-gateway
Length of output: 2416
🏁 Script executed:
cat -n core/src/test/java/com/predic8/membrane/core/resolver/ResolverMapCombineTest.java | sed -n '190,210p'Repository: membrane/api-gateway
Length of output: 795
🏁 Script executed:
# Also check if there are any other file URI tests that don't use hardcoded C:
rg -n "file:/" core/src/test/java/com/predic8/membrane/core/resolver/ResolverMapCombineTest.java | head -30Repository: membrane/api-gateway
Length of output: 1500
The hardcoded C: drive assumption will cause test failures on Windows systems with non-C: drives.
The code flow in FileUtil.resolve() → toFileURIString() → f.getAbsolutePath() uses the current working drive when resolving relative paths. On Windows, new File("/chi", "gnat").getAbsolutePath() returns the actual current drive (D:, E:, etc.), not C:\. These tests will fail when run from any drive other than C:.
This inconsistency is pre-existing (lines 72, 79, 86, 94, 102, 146 all hardcode C:), but the two changed tests extend it. Consider using the dynamically computed current variable (already used in relative-path tests) or a platform-aware approach for file URI tests to ensure portability.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/src/test/java/com/predic8/membrane/core/resolver/ResolverMapCombineTest.java`
around lines 66 - 67, The tests in ResolverMapCombineTest use hardcoded
"file:/C:/" URIs which fail on non-C drives; update the assertions that call
combine(...) and wl(...) to build expected file URIs dynamically using the
existing platform-aware helper/variable (e.g., the current variable used
elsewhere in the test) or by converting a java.io.File to a file URI (via
File#getAbsoluteFile or File#toURI) so the expected string matches the runtime
drive; adjust assertions referencing combine("file:/chi/elm","gnat") and other
hardcoded "file:/C:..." occurrences to use that computed URI instead.
1 participant
Summary by CodeRabbit