Skip to content

Fix path test on windows #2818

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.

Sign up for GitHub

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

Open
t-burch wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix path test on windows #2818

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions core/src/test/java/com/predic8/membrane/core/resolver/ResolverMapCombineTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ void relativeWithSpacePlusFile() {

@Test
void fileSingleSlashPlusFile() {
assertEquals("file:/chi/gnat", combine("file:/chi/elm","gnat"));
assertEquals(wl("file:/C:/chi/gnat",
"file:/chi/gnat"), combine("file:/chi/elm","gnat"));
Comment on lines +66 to +67
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "ResolverMap.java" -type f

Repository: membrane/api-gateway

Length of output: 136

🏁 Script executed:

rg -n "public.*combine" --type=java -A 30 | head -100

Repository: 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 -80

Repository: membrane/api-gateway

Length of output: 2952

🏁 Script executed:

find . -name "URIUtil.java" -o -name "FileUtil.java" | head -5

Repository: membrane/api-gateway

Length of output: 194

🏁 Script executed:

rg -n "pathFromFileURI|convertPath2FilePathString" --type=java -B 2 -A 10 | head -100

Repository: membrane/api-gateway

Length of output: 10905

🏁 Script executed:

# Check URIUtil class
rg -n "class URIUtil" --type=java -A 40

Repository: 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 8

Repository: 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 -150

Repository: 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 -150

Repository: 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 3

Repository: 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 -30

Repository: 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.

}

@Test
Expand Down Expand Up @@ -195,7 +196,10 @@ void fileSingleSlashPlusFileSpace() {

@Test
void filePlusPathSpace() {
assertEquals("file:/cock%20lock", combine("file:/chi","cock lock"));
assertEquals(wl(
"file:/C:/cock%20lock",
"file:/cock%20lock"),
combine("file:/chi","cock lock"));
}

@Test
Expand Down
Loading