FileUtilTest: Add test for Windows path

[sijie123]

Currently, FileUtilTest#isValidPath() only tests for isValidPath for a linux path. However, our users can also develop and use AB4 on a Windows PC. To provide assurance that our FileUtil#isValidPath(String) method works on Windows computers as well, we should add coverage to test for paths on Windows.

Hence, let's add another test to ensure that FileUtil#isValidPath returns the correct result for a valid path in the Windows naming convention.

[CanIHasReview-bot]

Click here to submit a new iteration when this PR is ready for review.

See this repository's contribution guide for more information.

[CanIHasReview-bot]

v1

@sijie123 submitted v1 for review.

• [v1 1/1] FileUtilTest: Add test for Windows path

(📚 Archive)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/976/1/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you wish to fetch this PR to.

[pyokagan]

FileUtilTest: Add test for Windows path

Currently, FileUtilTest#isValidPath() only tests for isValidPath for a
linux path.

I don't quite follow. valid/file/path is a valid Windows path as well, no? It is semantically the same as well.

Oh, it's also a valid Mac path as well :-P

However, our users can also develop and use AB4 on a
Windows PC. To provide assurance that our FileUtil#isValidPath(String)
method works on Windows computers as well, we should add coverage to
test for paths on Windows.

Hence, let's add another test to ensure that FileUtil#isValidPath
returns the correct result for a valid path in the Windows naming
convention.

[pyokagan]

I don't like the idea of the test since valid\file\path is not just a "Windows path", It is a valid Linux path as well. So, it's not correct to annotate them with the // Linux and // Windows comments.

Also, compared to valid/file/path, valid\file\path has different meanings on Windows and Linux. On Linux it just means "a file with the name valid\file\path", which is probably not the intention here.

Anyway, as the very sparse "invalid path" test case below hints, these are only simple smoke tests to check to see if isValidPath() is properly wired up to Paths.get(). It is assumed that Paths.get() is implemented correctly. So I don't really see the value in adding more tests.

[sijie123]

I think I was kinda loose on my Windows/Linux terminology use.
According to this Stack Overflow page, actually "a/b/c" is not a valid filesystem path on Windows, although it is valid as a URI.

In any case, I agree with you in that the method is essentially a hook to Paths.get(), so if we can assume that Paths.get() is implemented correctly, then this extra test certainly doesn't add value.

I will close this issue.