vsapsai added inline comments.

================
Comment at: 
test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp:257-258
     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);
----------------
vsapsai wrote:
> We need a comment explaining the intention of this check. Do you know what 
> are the values when the test fails?
> 
> I have another (not tested) interpretation of the test that consistently 
> compares write times with write times and access times with access times.
> 
> ```
>     TEST_CHECK(LastAccessTime(file) == file_access_time ||
>                LastWriteTime(file) == Clock::to_time_t(ftime2));
> ```
> 
> I am suspicious about the suggested condition `LastWriteTime(file) == 
> file_write_time` as I expect it to be false all the time.
After checking the last commit that touched this code 
https://reviews.llvm.org/rL278357 I'm not sure that was a mistake. Looks like 
the intention was indeed to check LastAccessTime.

What do you think about removing these LastAccessTime checks, including the one 
for `dir`? There is no function `last_access_time`, so we aren't testing the 
library but idiosyncrasies of file systems.


Repository:
  rCXX libc++

https://reviews.llvm.org/D47557



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to