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

[Volodymyr Sapsai via Phabricator via cfe-commits]

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