[libcxx-commits] [PATCH] D102357: [libcxx] [test] Explain an XFAIL LIBCXX-WINDOWS-FIXME and convert into "UNSUPPORTED"

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision.
Mordante added a comment.
This revision is now accepted and ready to land.

Some small nits other than that LGTM. The UNSUPPORTED change shouldn't break anything, but please keep eye out. Alternatively give it another CI run and land it after the build passes.



================
Comment at: libcxx/test/libcxx/input.output/filesystems/class.directory_entry/directory_entry.mods/last_write_time.pass.cpp:13
+// This test relies on calling functions from the libcxx internal header
+// filesystem_common.h; the windows implementation uses different
+// internals and doesn't provide the same set_file_times function as for
----------------
Please capitalize Windows.


================
Comment at: libcxx/test/libcxx/input.output/filesystems/class.directory_entry/directory_entry.mods/last_write_time.pass.cpp:16
+// other platforms.
+// XFAIL: windows
 
----------------
curdeius wrote:
> mstorsjo wrote:
> > curdeius wrote:
> > > Your patch description said that it was going to be converted into UNSUPPORTED, right?
> > Oh, right. What do you think, which marking would be better here?
> No strong feelings here. I'd probably go for unsupported. Your comment clearly explains why it is so IMO. Also, unless you think it will change in a foreseeable future, it makes no sense to me to XFAIL.
> But, yeah, I leave it to you (and other reviewers).
Reading the comment I think UNSUPPORTED would be better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102357/new/

https://reviews.llvm.org/D102357