mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Raluca Miclea <rmic...@cloudbasesolutions.com>
Subject Re: Review Request 63245: Ported and enabled Os Tests on Windows.
Date Thu, 26 Oct 2017 14:54:35 GMT


> On Oct. 24, 2017, 6:55 p.m., Andrew Schwartzmeyer wrote:
> > This review request should really be at least three separate patches. THere is work
to be done to fix several of these tests, and right now they're all mixed together.

should I discard this commit and make separate ones for each issue?


- Raluca


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


On Oct. 24, 2017, 9:30 a.m., Raluca Miclea wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63245/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2017, 9:30 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-3441
>     https://issues.apache.org/jira/browse/MESOS-3441
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> TEST_F(OsTest, Libraries)
> The test failed because no environmental variable was provided for
> Windows. I used "PATH" as environmental variable on Windows since
> there's no default variable where the linker should look into while
> linking dynamic libraries/shared libraries. (like LD_LIBRARY_PATH on Unix/Linux)
> 
> TEST_F(OsTest, Sleep)
> The test hanged because of the last assertion:
> ASSERT_ERROR(os::sleep(Milliseconds(-10)));
> Apparently, it didn't handle a negative sleep duration. By simply
> returning an error value when the duration is negative the problem was
> solved.
> 
> TEST_F(OsTest, SYMLINK_Size)
> First issue was this assertion:
> EXPECT_SOME_EQ(size, os::stat::size(file,FollowSymlink::DO_NOT_FOLLOW_SYMLINK));
> It fails everytime because <file> is not a symbolic link. When calling
> os:stat:size with argument <follow> set to "DO_NOT_FOLLOW_SYMLINK", it firstly
> checks whether the path it receives is a symlink in order to get the symlink data
> for that path. If it's not a symlink, it returns an error.
> If you are supposed to be able to test the size of a non-link file in
> the context of not-following-a-link, the program should return the
> size of the non-link file instead of returning an error upon determining the
> nature of the path as not being a link file path. Right now, the assertion
> is not relevant since you'll get an error if you supply it a non-link file
> path rather thana symlink.
> 
> TEST_F(OsTest, SYMLINK_Realpath)
> The issue was that the symlink resolved to itself but it was tested
> against the path of the target file. Once the path of the target file
> was replaced by the path of the symlink, the test passed.
> 
> 
> Diffs
> -----
> 
>   3rdparty/stout/include/stout/os.hpp 1a81db61faa55d7043d75a012fe1e66b49bf601c 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 0db6d482ad19897db33d334bffe4b294da11a36f

>   3rdparty/stout/include/stout/windows/os.hpp 09ddec6d69472cd13b453fe1a77fdbe343fc23c8

>   3rdparty/stout/tests/os_tests.cpp 959b02d08f06481a64ef1a39acf23b062d5dc805 
> 
> 
> Diff: https://reviews.apache.org/r/63245/diff/1/
> 
> 
> Testing
> -------
> 
> Modified tests:
> TEST_F(OsTest, Libraries)
> TEST_F(OsTest, Sleep)
> TEST_F(OsTest, SYMLINK_Size)
> TEST_F(OsTest, SYMLINK_Realpath)
> 
> 
> Thanks,
> 
> Raluca Miclea
> 
>


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