[circt-verilog-lsp-server] Simplify MaxCapForcesFlushDuringContinuousTyping by removing bakground thread

[uenoku]

This commit refactors the MaxCapForcesFlushDuringContinuousTyping test to eliminate the background thread used for simulating continuous typing. The cap is increased to pass valgrind (at least on CI environment...). Not proud of the change and we should seek a better way to reduce flaky parts.

Fix #9075.

CI: https://github.com/llvm/circt/actions/runs/18416091701/job/52480002926

[Scheremo]

This commit refactors the MaxCapForcesFlushDuringContinuousTyping test to eliminate the background thread used for simulating continuous typing. The cap is increased to pass valgrind (at least on CI environment...). Not proud of the change and we should seek a better way to reduce flaky parts.

Fix #9075.

CI: https://github.com/llvm/circt/actions/runs/18416091701/job/52480002926

Thanks for pointing this out! In hindsight this test looks particularly fragile, but probably most of these unit tests are too optimistic with assuming scheduling stability.
I think we can avoid time-based races by factoring out std::chrono::steady_clock::now and replace it with a callable that we can manually override for testing. That way we can get rid of threading in unittesting altogether. E.g.:

using Clock = std::chrono::steady_clock;
using NowFn = std::function<Clock::time_point()>;
class PendingChangesMap {
public:
  explicit PendingChangesMap(NowFn now = []{ return Clock::now(); })
      : nowFn(std::move(now)) {}
...
private:
  NowFn nowFn;


struct ManualClock {
  using Clock = std::chrono::steady_clock;
  Clock::time_point t = Clock::time_point{};
  Clock::time_point now() const { return t; }
  void advanceMs(int ms) { t += std::chrono::milliseconds(ms); }
};

TEST(PendingChangesMapTest, MaxCapForcesFlushDuringContinuousTyping) {
  ManualClock mc;
  PendingChangesMap pcm(2, [&]{ return mc.now(); });

  DebounceOptions opt;
  opt.disableDebounce = false;
  opt.debounceMinMs = 100;
  opt.debounceMaxMs = 200;

  const auto *key = "d.sv";
  auto p = makeChangeParams(key, 1);
  pcm.enqueueChange(p);

  for (int v = 2; v < 20; ++v) {
    mc.advanceMs(50);                            // time passes
    pcm.enqueueChange(makeChangeParams(key, v)); // “user typed”
  }

  CallbackCapture first;
  pcm.debounceAndThen(p, opt, [&](std::unique_ptr<PendingChanges> r) {
    first.set(std::move(r));
  });
  mc.advanceMs(100);
  ASSERT_TRUE(first.waitFor(std::chrono::milliseconds(50)));
  EXPECT_EQ(first.got, nullptr);

  mc.advanceMs(250); // > 200ms cap since last schedule/key activity
  CallbackCapture second;
  auto p2 = makeChangeParams(key, /*version=*/999);
  pcm.debounceAndThen(p2, opt, [&](std::unique_ptr<PendingChanges> r) {
    second.set(std::move(r));
  });

  ASSERT_TRUE(second.waitFor(std::chrono::milliseconds(50)));
  ASSERT_TRUE(second.got != nullptr);
  EXPECT_GE(second.got->version, 2);
  EXPECT_GE(second.got->changes.size(), 1u);
}

I can take a stab at that; if you prefer we can also revert the LSP commit and re-land once we have a better solution.

[Scheremo]

I think this is fine to mitigate the problem of failing CI.
Medium-term we should probably switch to manual clocking in CI to avoid threading issues.

[uenoku]

Thank you for the review! Overriding clock in tests makes sense. Actually I explored a bit of that direction but it was challenging to model sleep in the async thread

circt/lib/Tools/circt-verilog-lsp-server/Utils/PendingChanges.cpp

Lines 89 to 91 in 0dd8d0d

	if (options.debounceMinMs > 0)
	std::this_thread::sleep_for(
	std::chrono::milliseconds(options.debounceMinMs));

with a manual clock. Maybe some inconsistency is fine in tests though.