[libcxx-commits] [PATCH] D143816: [libc++] Fixes a flaky test.

[Louis Dionne via Phabricator via libcxx-commits]

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

I think I like those changes, however I am wondering -- does the test still fail as reliably with those changes? Did you run it on the backdeployment host with those changes and you could see it fail pretty much as often as before? If so, this LGTM. If this makes the test not reproduce (or reproduce much less frequently), then we're reducing its effectiveness and I'd argue we shouldn't.

LGTM assuming this does not power-down the test.



================
Comment at: libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit_lwg3343.pass.cpp:78-79
+        //
+        // Keep track of how often that happens, when too often the test needs
+        // to be improved.
+        if(threads_active == 0)
----------------



================
Comment at: libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit_lwg3343.pass.cpp:18
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{.+}}
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx11.{{.+}}
+
----------------
Mordante wrote:
> @ldionne please verify whether these targets are correct. These are the same values I use for the format backdeployment tests.
Yeah, that sounds fine to me. I think 12.0 would fail too, but this can be fixed in bulk when we add testing for 12.0 in backdeployment. Other tests will also need fixing anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143816