Issue 5610 - Deal with a '/' base path

Issue 5610 - Deal with a '/' base path

[hub, 2017-09-07 17:10:33]

This include tests.

[sergei, 2017-09-08 08:11:40]

Good, merely a couple of things in tests.

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
File test/DefaultFileSystem.cpp (right):

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
test/DefaultFileSystem.cpp:39: class BasePathTest : public ::testing::Test
It seems this fixture is not required, one can simple use TEST instead of TEST_F

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
test/DefaultFileSystem.cpp:84: #define SLASH_STRING "/"
These defines are already available as PATH_SEPARATOR from
"../src/DefaultFileSystem.h" which is included in this file.

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
test/DefaultFileSystem.cpp:88: TEST_F(BasePathTest, BasePathTest)
What about renaming it into something emphasizing that it's a test of Resolve of
DefaultFileSystemSync?

[hub, 2017-09-08 13:10:26]

updated patch

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
File test/DefaultFileSystem.cpp (right):

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
test/DefaultFileSystem.cpp:39: class BasePathTest : public ::testing::Test
On 2017/09/08 08:11:40, sergei wrote:
> It seems this fixture is not required, one can simple use TEST instead of
TEST_F

Done.

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
test/DefaultFileSystem.cpp:84: #define SLASH_STRING "/"
On 2017/09/08 08:11:40, sergei wrote:
> These defines are already available as PATH_SEPARATOR from
> "../src/DefaultFileSystem.h" which is included in this file.

but they are single char, while this is a string.

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
test/DefaultFileSystem.cpp:88: TEST_F(BasePathTest, BasePathTest)
On 2017/09/08 08:11:40, sergei wrote:
> What about renaming it into something emphasizing that it's a test of Resolve
of
> DefaultFileSystemSync?

Done.

[sergei, 2017-09-08 13:20:19]

LGTM

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
File test/DefaultFileSystem.cpp (right):

https://codereview.adblockplus.org/29538640/diff/29538641/test/DefaultFileSys...
test/DefaultFileSystem.cpp:84: #define SLASH_STRING "/"
On 2017/09/08 13:10:26, hub wrote:
> On 2017/09/08 08:11:40, sergei wrote:
> > These defines are already available as PATH_SEPARATOR from
> > "../src/DefaultFileSystem.h" which is included in this file.
> 
> but they are single char, while this is a string.

Acknowledged.