Skip to content
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

Files that have whitespace in their paths lead to test failures #435

Closed
amrut-prabhu opened this issue Dec 17, 2018 · 8 comments · Fixed by #1556
Closed

Files that have whitespace in their paths lead to test failures #435

amrut-prabhu opened this issue Dec 17, 2018 · 8 comments · Fixed by #1556

Comments

@amrut-prabhu
Copy link

Bug:
For example, the following exception is encountered due to whitespace in Abc Xyz, which is in the file path to the test file standardBlameOutput.txt.

   Path STRINGS_UTIL_TEST_DIRECTORY = ...

20 Path gitBlameStandardOutputFile =
21                Paths.get(STRINGS_UTIL_TEST_DIRECTORY.toString(), "standardBlameOutput.txt");
22 List<String> lines = Files.readAllLines(gitBlameStandardOutputFile);
java.nio.file.NoSuchFileException: C:\Users\Abc%20Xyz\git\RepoSense\out\test\resources\StringsUtilTest\standardBlameOutput.txt

	at sun.nio.fs.WindowsException.translateToIOException(WindowsException.java:79)
	...
	at java.nio.file.Files.readAllLines(Files.java:3242)
	at reposense.util.StringsUtilTest.filterText_standardInput_success(StringsUtilTest.java:22)

Cause:
The exception occurs since Path.toString() returns the path with the spaces replaced by %20. But, java.io.File accepts spaces in the String pathname, not %20.

Fix:
I think it can be fixed by replacing such instances by
String decodedPath = URLDecoder.decode(Path.toString(), "UTF-8");

@yamidark
Copy link
Contributor

Yes, we probably need to change all the Path that uses ClassLoader in test to use the decoder to handle this case.

However, there are currently too many of such Path that uses the ClassLoader, which is already very long, and it is too troublesome to go edit all of them.

Probably want a different issue to refactor them all out into a common utility method 1st for ease of future usage, and then update that method to use the decoder.

@eugenepeh Your opinion on this?

@eugenepeh
Copy link
Member

eugenepeh commented Dec 23, 2018

Probably want a different issue to refactor them all out into a common utility method 1st for ease of future usage, and then update that method to use the decoder.

I think it can be done in a single PR, since updating the method would probably be just a few line change afterwards.

@fzdy1914
Copy link
Member

fzdy1914 commented Mar 3, 2019

When I work around with this issue, only adding URLDecoder is not enough. For the directories in Argsparser, they will be passed in as an argument. An argument does not allow to have whitespace within it.

@damithc damithc added the p.Low label May 16, 2019
@Tejas2805
Copy link
Contributor

@jamessspanggg your PR fixed this issue too right? Or was it just for spaces in the file name and not the file path?

@chan-j-d
Copy link
Contributor

Hi, I would like to work on this issue even though it is marked as ToVerifyRelevance. I believe I already have most of the fix after spending some time on it as the project couldnt be built on my own computer because of the issue.

@fzdy1914
Copy link
Member

Hi, I would like to work on this issue even though it is marked as ToVerifyRelevance. I believe I already have most of the fix after spending some time on it as the project couldnt be built on my own computer because of the issue.

Go ahead.

@damithc
Copy link
Collaborator

damithc commented Jun 26, 2021

Go ahead @chan-j-d

@HCY123902
Copy link
Contributor

HCY123902 commented Jun 26, 2021

Hi, I would like to work on this issue even though it is marked as ToVerifyRelevance. I believe I already have most of the fix after spending some time on it as the project couldnt be built on my own computer because of the issue.

I previously worked on this issue. I think part of the reason why it is marked as ToVerifyRelevance is that the problem with Path.toString() does not exist when used along with Paths.get() in higher version of Java. Additionally, the suggestion of using URLDecoder.decode() may bring problem when parsing path containing special character such as the plus sign. You may want to pay attention to this special case

dcshzj pushed a commit that referenced this issue Jul 9, 2021
The test suite will convert spaces in URLs into %20, which can lead to
test case failures. Java API recommends managing encoding and decoding
of URLs by using URI, which would resolve the issue with spaces.

This could save new developers some trouble when encountering test
failures while attempting to build the project locally for the first
time.

Let's fix the issues caused by whitespace in the test cases.
@dcshzj dcshzj removed this from To verify relevance in Roadmap Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants