> On April 25, 2017, 11:16 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/stout/include/stout/path.hpp
> > Lines 79-86 (patched)
> > <https://reviews.apache.org/r/58673/diff/2/?file=1699763#file1699763line79>
> >
> > I think this got a bit out of date. We settled on `\...` being an absolute path,
not just `\?...` (per below implementation).
>
> Jeff Coffler wrote:
> I didn't have a test for this specifically. The above is accurate, though. The 1-3
cases are for files on disk. Network shares are a special case and, while technically an absolute
path, isn't related to files on disk.
>
> That said, the function does implement this properly. The comment isn't out of date.
The note was just to point out that we handled this as well.
Sorry, I mean that your third bullet should read `3. "\..."` instead of `3. "\?..."`.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58673/#review173000
-----------------------------------------------------------
On April 25, 2017, 6:24 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58673/
> -----------------------------------------------------------
>
> (Updated April 25, 2017, 6:24 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-5937
> https://issues.apache.org/jira/browse/MESOS-5937
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added 3rdparty/stout/tests/path_tests.cpp tests to Windows platform,
> but disabled tests that did not pass. Enabled absolute path tests,
> adding tests that were appropriate for the Windows platform.
>
> Note that, for Windows, absolute paths may not be valid. For example,
> a path like "\\?\abc:file.txt" is not valid. In this case, the
> path::absolute method is undefined; the expectation is that it is
> called with valid paths (absolute or relative, but valid).
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/flags/parse.hpp 65edd86372596c2107e9f29cf27301e025e6620e
> 3rdparty/stout/include/stout/path.hpp 2d2088aadfa1ea82c59424242671c4fb655dede1
> 3rdparty/stout/tests/CMakeLists.txt 4bbe713f259e7858d423dcb33956d41e62a915eb
> 3rdparty/stout/tests/flags_tests.cpp e2681f8f68f6478d4c8a20c1e75ddb050d16b56d
> 3rdparty/stout/tests/path_tests.cpp 0490d93908566c46a10d91b05790e5a7f2f289bc
>
>
> Diff: https://reviews.apache.org/r/58673/diff/2/
>
>
> Testing
> -------
>
> Passes `make check` on the Linux platform.
>
> On Windows, the FlagsFileTest.JSONFile now passes, along with new PathTest.Absolute tests
that I added due to new path::absolute handling on the Windows platform.
>
> PS C:\mesos> .\build\3rdparty\stout\tests\Debug\stout-tests.exe --gtest_filter="*FlagsFileTest*"
> Note: Google Test filter = *FlagsFileTest*-
> [==========] Running 2 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 2 tests from FlagsFileTest
> [ RUN ] FlagsFileTest.JSONFile
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> W0425 10:05:20.959357 1060440 parse.hpp:97] Specifying an absolute filename to read a
command line option out of without using 'file:// is deprecated and will be removed in a future
release. Simply adding 'file://' to the beginning of the path should eliminate this warning.
> [ OK ] FlagsFileTest.JSONFile (9 ms)
> [ RUN ] FlagsFileTest.FilePrefix
> [ OK ] FlagsFileTest.FilePrefix (6 ms)
> [----------] 2 tests from FlagsFileTest (18 ms total)
>
> [----------] Global test environment tear-down
> [==========] 2 tests from 1 test case ran. (23 ms total)
> [ PASSED ] 2 tests.
> PS C:\mesos> .\build\3rdparty\stout\tests\Debug\stout-tests.exe --gtest_filter="*PathTest*"
> Note: Google Test filter = *PathTest*-
> [==========] Running 2 tests from 1 test case.
> [----------] Global test environment set-up.
> [----------] 2 tests from PathTest
> [ RUN ] PathTest.Absolute
> [ OK ] PathTest.Absolute (1 ms)
> [ RUN ] PathTest.Comparison
> [ OK ] PathTest.Comparison (0 ms)
> [----------] 2 tests from PathTest (2 ms total)
>
> [----------] Global test environment tear-down
> [==========] 2 tests from 1 test case ran. (6 ms total)
> [ PASSED ] 2 tests.
>
> YOU HAVE 4 DISABLED TESTS
>
> PS C:\mesos>
>
>
> Thanks,
>
> Jeff Coffler
>
>
|