Mailing-List: contact reviews-help@mesos.apache.org; run by ezmlm
Precedence: bulk
Reply-To: reviews@mesos.apache.org
Content-Type: multipart/alternative;
 boundary="===============6635660672021588089=="
MIME-Version: 1.0
Subject: Review Request 63245: Ported and enabled Os Tests on Windows.
From: Raluca Miclea <rmiclea@cloudbasesolutions.com>
To: Andrew Schwartzmeyer <andrew@schwartzmeyer.com>
Cc: Raluca Miclea <rmiclea@cloudbasesolutions.com>,
 mesos <reviews@mesos.apache.org>
Date: Tue, 24 Oct 2017 09:27:44 -0000
Message-ID: <20171024092744.64628.50443@reviews-vm2.apache.org>
Auto-Submitted: auto-generated
Sender: Raluca Miclea <noreply@reviews.apache.org>
Reply-To: Raluca Miclea <rmiclea@cloudbasesolutions.com>
archived-at: Tue, 24 Oct 2017 09:27:52 -0000

--===============6635660672021588089==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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

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_TEMP_DISABLED_ON_WINDOWS(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_TEMP_DISABLED_ON_WINDOWS(OsTest, SYMLINK_Realpath)


Thanks,

Raluca Miclea


--===============6635660672021588089==--