[PATCH] D125103: [clangd] Speed up a slow sleeping testcase.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 6 10:35:18 PDT 2022


ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Many thanks for improving the running time, this slow test has bugged me since the day I first ran it.
I'd even be happy to remove it completely. It's slow, possibly flaky, so IMO the corresponding check does not carry its weight.

I'm not sure if we introduced a new race condition there that will cause flaky test, see the comment.
Otherwise LGTM.



================
Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:272
+                  [&](std::vector<Diag>) {
+                    ADD_FAILURE()
+                        << "auto should have been discarded (dead write)";
----------------
This is potentially a race in the test that we did not have before, right?
There is probably much less work in the main test thread, but we probably can still see the failure if we are unlucky with how the OS schedules us.

My stance is it's better to have no tests for certain behaviours than flaky tests.
But if you find it useful let's try to run it and see whether my suspicions are unfounded, don't want to block the change on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125103



More information about the cfe-commits mailing list