mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Coffler <j...@taltos.com>
Subject Re: Review Request 58673: Fix FlagsFileTest to check for absolute path properly on Windows.
Date Mon, 01 May 2017 21:01:16 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58673/
-----------------------------------------------------------

(Updated May 1, 2017, 9:01 p.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.


Changes
-------

I understand what Joe was asking for, and resolved that in a second commit. Already posted
for review.


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 (updated)
-----

  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/4/

Changes: https://reviews.apache.org/r/58673/diff/3-4/


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message