Skip to content

fix(tests): resolve remaining flaky tests after Vitest migration#3242

Open
vseehausen wants to merge 3 commits into
mainfrom
fix/wa-1753-remaining-flaky-tests
Open

fix(tests): resolve remaining flaky tests after Vitest migration#3242
vseehausen wants to merge 3 commits into
mainfrom
fix/wa-1753-remaining-flaky-tests

Conversation

@vseehausen

Copy link
Copy Markdown
Member

Summary

Fixes three tests that kept flaking after #3139. All three were visible in Datadog CI Pipeline Health but missed by our own flaky-test report, whose log parser is Jest-specific and went blind once the suite moved to Vitest (#3136).

  • name.buildernameBuilder() sanitized (trimmed) the faker name and then truncated to NAME_MAX_LENGTH; the cut could land right after a space and re-introduce trailing whitespace, so sanitizeName(name) !== name. Now re-sanitizeName() after truncation so the result is a fixed point.
  • bridge-name.entity.spec — the "unknown bridge name" case used faker.word.noun(), which can itself be a bridge name (omni, polygon, across, gnosis, …), making safeParse succeed and the assertion flip to expected false to deeply equal …. Now resamples until the word is genuinely not a bridge name.
  • Integration specs (61 files)afterEach/afterAll called await app.close() unguarded, so any beforeEach failure left app undefined and every test in the file reported Cannot read properties of undefined (reading 'close') instead of the real error (the "27 tests fail at teardown" cascade). Guarded with await app?.close().

Testing

  • yarn test:unit for name.builder + bridge-name.entity → pass
  • biome check clean; tsc introduces no new errors

Related

Three tests kept flaking after #3139 (all surfaced via Datadog CI Pipeline
Health, which our log-scraping report missed once the suite moved to Vitest):

- name.builder: nameBuilder() sanitized (trimmed) the faker name and THEN
  truncated to NAME_MAX_LENGTH; the cut could land right after a space and
  re-introduce trailing whitespace, so sanitizeName(name) !== name. Re-sanitize
  after truncation so the result is a fixed point.
- bridge-name.entity.spec: the 'unknown bridge name' case used
  faker.word.noun(), which can itself be a bridge name (omni, polygon, across,
  gnosis, ...), making safeParse succeed and the assertion flip. Resample until
  the word is genuinely not a bridge name.
- integration specs: afterEach/afterAll called 'await app.close()' unguarded,
  so any beforeEach failure left app undefined and every test in the file
  reported 'Cannot read properties of undefined (reading close)' instead of the
  real error. Guarded with 'await app?.close()' across all integration specs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vseehausen vseehausen requested a review from a team as a code owner July 8, 2026 15:45
@vseehausen

This comment was marked as resolved.

@claude

This comment was marked as resolved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@vseehausen

This comment was marked as resolved.

@vseehausen

This comment was marked as resolved.

@PooyaRaki

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown

Claude finished @PooyaRaki's task in 1m 33s —— View job


Claude PR Review (re-review)

  • Check follow-up commit ce81716 (dropped if (app) guards)
  • Verify name.builder.ts fix
  • Verify bridge-name.entity.spec.ts fix
  • Sample integration specs for consistency
  • Post final review

Follow-up looks good — LGTM. No further blockers from me.

Verified

  • ce81716 (if (app) guards dropped)grep for if (app) across **/*.integration.spec.ts returns zero matches; the two touched files (feature-flag.service.integration.spec.ts:23, messages.controller.integration.spec.ts:49) now use the plain await app?.close() pattern used everywhere else in the PR. Consistent across all 63 files.
  • name.builder.ts — re-sanitizeName() after codepoint-aware truncation is correct; [...padded].slice(0, NAME_MAX_LENGTH).join('') handles surrogate pairs safely, and the second sanitize normalizes any whitespace re-introduced at the cut boundary.
  • bridge-name.entity.spec.ts — noted your reasoning for keeping the unbounded while loop (avoiding a deterministic fallback that undermines the "random non-bridge noun" intent). Fair trade-off given faker.word.noun() draws from a very large vocab vs. ~20 bridge names — non-termination is astronomically unlikely with a seeded faker.

Nits (non-blocking, informational)

None new. Previously-raised #3 (theoretical NAME_MIN_LENGTH violation after trailing-whitespace trim) remains not reachable in practice with the current SAMPLE_NAMES + faker.person.fullName() inputs.

Ship it.
• branch fix/wa-1753-remaining-flaky-tests

@vseehausen vseehausen requested a review from PooyaRaki July 9, 2026 16:28
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.

2 participants