[PATCH] D47557: Filesystem tests: un-confuse write time

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
jfb added reviewers: EricWF, mclow.lists, aemerson.
Herald added subscribers: cfe-commits, christof.

The filesystem test was confused about access versus write / modification time. The spec says:

  file_time_type last_write_time(const path& p, error_code& ec) noexcept;
  Returns: The time of last data modification of p, determined as if by the value of the POSIX stat structure member st_mtime obtained as if by POSIX stat(). The signature with argument ec returns file_time_type::min() if an error occurs.

The test was looking at st_atime, not st_mtime, when comparing the result from last_write_time. That was probably due to using a pair instead of naming things nicely or using types. I opted to rename things so it's clearer.

This caused test bot failures.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47557

Files:
  test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp


Index: test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
===================================================================
--- test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
+++ test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp
@@ -32,7 +32,9 @@
 
 using namespace fs;
 
-std::pair<std::time_t, std::time_t> GetTimes(path const& p) {
+struct Times { std::time_t access, write; };
+
+Times GetTimes(path const& p) {
     using Clock = file_time_type::clock;
     struct ::stat st;
     if (::stat(p.c_str(), &st) == -1) {
@@ -48,11 +50,11 @@
 }
 
 std::time_t LastAccessTime(path const& p) {
-    return GetTimes(p).first;
+    return GetTimes(p).access;
 }
 
 std::time_t LastWriteTime(path const& p) {
-    return GetTimes(p).second;
+    return GetTimes(p).write;
 }
 
 std::pair<std::time_t, std::time_t> GetSymlinkTimes(path const& p) {
@@ -228,11 +230,10 @@
     const path dir = env.create_dir("dir");
 
     const auto file_times = GetTimes(file);
-    const std::time_t file_access_time = file_times.first;
-    const std::time_t file_write_time = file_times.second;
+    const std::time_t file_write_time = file_times.write;
     const auto dir_times = GetTimes(dir);
-    const std::time_t dir_access_time = dir_times.first;
-    const std::time_t dir_write_time = dir_times.second;
+    const std::time_t dir_access_time = dir_times.access;
+    const std::time_t dir_write_time = dir_times.write;
 
     file_time_type ftime = last_write_time(file);
     TEST_CHECK(Clock::to_time_t(ftime) == file_write_time);
@@ -253,8 +254,8 @@
 
     TEST_CHECK(ftime2 > ftime);
     TEST_CHECK(dtime2 > dtime);
-    TEST_CHECK(LastAccessTime(file) == file_access_time ||
-               LastAccessTime(file) == Clock::to_time_t(ftime2));
+    TEST_CHECK(LastWriteTime(file) == file_write_time ||
+               LastWriteTime(file) == Clock::to_time_t(ftime2));
     TEST_CHECK(LastAccessTime(dir) == dir_access_time);
 }
 
@@ -301,7 +302,7 @@
     };
     for (const auto& TC : cases) {
         const auto old_times = GetTimes(TC.p);
-        file_time_type old_time(Sec(old_times.second));
+        file_time_type old_time(Sec(old_times.write));
 
         std::error_code ec = GetTestEC();
         last_write_time(TC.p, TC.new_time, ec);
@@ -318,7 +319,7 @@
                 TEST_CHECK(got_time <= TC.new_time + Sec(1));
                 TEST_CHECK(got_time >= TC.new_time - Sec(1));
             }
-            TEST_CHECK(LastAccessTime(TC.p) == old_times.first);
+            TEST_CHECK(LastAccessTime(TC.p) == old_times.access);
         }
     }
 }
@@ -348,10 +349,10 @@
     file_time_type  got_time = last_write_time(sym);
     std::time_t got_time_t = Clock::to_time_t(got_time);
 
-    TEST_CHECK(got_time_t != old_times.second);
+    TEST_CHECK(got_time_t != old_times.write);
     TEST_CHECK(got_time_t == new_time_t);
     TEST_CHECK(LastWriteTime(file) == new_time_t);
-    TEST_CHECK(LastAccessTime(sym) == old_times.first);
+    TEST_CHECK(LastAccessTime(sym) == old_times.access);
     TEST_CHECK(GetSymlinkTimes(sym) == old_sym_times);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D47557.149199.patch
Type: text/x-patch
Size: 3193 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180530/52cb0d88/attachment.bin>