Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 2, 2026

The timer.test.ts suite was intermittently failing in CI due to timing-sensitive test assertions. The Timer class was dead code—only used in its own tests.

Changes:

  • Removed packages/core/src/util/timer.baktest.ts (renamed test file)
  • Removed packages/core/src/util/timer.ts (Timer class implementation)
  • Updated packages/core/src/util/wait-until-event.util.ts to remove Timer from type union

The Timer type was only used for TypeScript type checking in waitUntilEvent, not runtime functionality. All 233 test suites pass.

Original prompt

This section details on the original issue you should resolve

<issue_title>Timer tests is flakey: timer.test.ts</issue_title>
<issue_description>### Where did this happen?

No response

Expected Behavior

When running the test suite, all tests should pass predictably

Current Behavior

This test intermittently fails.

Here is an example of a run when it failed: https://github.com/SwitchbackTech/compass/actions/runs/20394573058/job/58608292499

In the subsequent PR, it passed fine.

Failure output:


Summary of all failing tests
FAIL packages/core/src/util/timer.test.ts (15.322 s)
  ● Timer › tick event › emits tick events at specified interval

    thrown: "Exceeded timeout of 5000 ms for a test while waiting for `done()` to be called.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      157 |
      158 |   describe("tick event", () => {
    > 159 |     it("emits tick events at specified interval", (done) => {
          |     ^
      160 |       const start = dayjs().add(2, "seconds");
      161 |       const startDate = start.toDate();
      162 |       const endDate = start.add(1, "second").toDate();

      at it (packages/core/src/util/timer.test.ts:159:5)
      at describe (packages/core/src/util/timer.test.ts:158:3)
      at Object.describe (packages/core/src/util/timer.test.ts:5:1)


Test Suites: 1 failed, 207 passed, 208 total
Tests:       1 failed, 2 skipped, 1778 passed, 1781 total
Snapshots:   0 total
Time:        110.046 s
Ran all test suites in 4 projects.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Steps to Reproduce

  1. Re-run tests frequently. This might require doing so in separate PRs, as it might only happen in CI due to the resource constraints

Possible Solution

Fix the test. Or, if the the functionality isn't needed, remove it

Context

This slows down devs and discourages contributors</issue_description>

<agent_instructions>just remove timerbak.test and timer.ts and any other dead code related</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix flakey timer tests in timer.test.ts Remove flaky timer tests and dead code Jan 2, 2026
Copilot AI requested a review from tyler-dane January 2, 2026 15:52
@tyler-dane tyler-dane marked this pull request as ready for review January 2, 2026 21:58
Copilot AI review requested due to automatic review settings January 2, 2026 21:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the flaky timer.test.ts suite and its associated dead code. The Timer class was only used in its own tests and not referenced elsewhere in the codebase, making it safe to remove.

Key changes:

  • Removed the Timer class implementation (timer.ts)
  • Removed the flaky test file (timer.baktest.ts)
  • Updated type union in waitUntilEvent to remove Timer reference

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
packages/core/src/util/timer.ts Deleted the 174-line Timer class implementation that was only used in tests
packages/core/src/util/timer.baktest.ts Deleted the 273-line flaky test file containing timing-sensitive assertions
packages/core/src/util/wait-until-event.util.ts Removed Timer from the type union (type-only change, no runtime impact)

The changes look good! The Timer class and its tests have been cleanly removed, and the only reference to Timer in the codebase was a type annotation in waitUntilEvent. The removal is appropriate since:

  1. No other files import or use the Timer class (verified by searching the codebase)
  2. The type union change in waitUntilEvent is correct—Timer was only used for TypeScript type checking, not runtime functionality
  3. The tests in wait-until-event.util.test.ts continue to work with EventEmitter without needing Timer
  4. This resolves the flaky test issue described in the original issue

No issues found to report.

@tyler-dane tyler-dane changed the title Remove flaky timer tests and dead code chore(backend): Remove flaky timer tests and dead code Jan 2, 2026
@tyler-dane tyler-dane changed the title chore(backend): Remove flaky timer tests and dead code chore(core): Remove flaky timer tests and dead code Jan 2, 2026
@tyler-dane tyler-dane merged commit 34b3925 into main Jan 2, 2026
11 checks passed
@tyler-dane tyler-dane deleted the copilot/fix-flakey-timer-tests branch January 2, 2026 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete dead code and flakey tests: timer.ts & timer.test.ts

2 participants