Skip to content

test: fix two flaky tests that fail intermittently in CI#3844

Merged
SmittieC merged 1 commit into
mainfrom
cs/fix_failing_test
Jul 17, 2026
Merged

test: fix two flaky tests that fail intermittently in CI#3844
SmittieC merged 1 commit into
mainfrom
cs/fix_failing_test

Conversation

@SmittieC

Copy link
Copy Markdown
Contributor

Product Description

No change — test-only fixes.

Technical Description

Two tests fail intermittently in CI:

  • test_remove_start_end_nodes asserted on pipeline.node_set.all()[0] / [1], but Node has no Meta.ordering, so Postgres returns rows in arbitrary heap order after remove_all_start_end_nodes deletes and rewrites rows. Replaced the positional asserts with a set comparison of flow_ids, which is what the test actually meant to check.
  • test_prune_stale_clients slept real wall-clock time (0.4s + 0.1s) against a 0.5s stale timeout, so on a slow runner both clients went stale and got pruned (assert 0 == 1). The sleeps are gone; the test now backdates the first client's timestamp past the timeout, making it deterministic.

Docs and Changelog

  • This PR requires docs/changelog update

🤖 Generated with Claude Code

- test_remove_start_end_nodes asserted positional order on an unordered
  queryset (Node has no Meta.ordering); compare flow_id sets instead.
- test_prune_stale_clients raced real wall-clock sleeps against the
  stale timeout; backdate the first client's timestamp instead of
  sleeping.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Two tests were revised for deterministic behavior. The pipeline test now compares the remaining node flow_id values without relying on query ordering. The Langfuse client pruning test directly backdates one client timestamp instead of waiting with time.sleep, leaving the second client fresh before pruning.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately summarizes the main change: fixing two flaky CI tests.
Description check ✅ Passed The description matches the template well and includes the required product and technical summaries plus the changelog checkbox.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@SmittieC
SmittieC marked this pull request as ready for review July 17, 2026 12:56
@SmittieC
SmittieC merged commit 96dcd06 into main Jul 17, 2026
19 checks passed
@SmittieC
SmittieC deleted the cs/fix_failing_test branch July 17, 2026 12:56
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.

1 participant