Skip to content

Fix flaky TestArchiveInquirer_RunProgressReport#228

Merged
wsodsong merged 3 commits into
mainfrom
ws/fix-flacky-testarchiveinquirer
Jun 30, 2026
Merged

Fix flaky TestArchiveInquirer_RunProgressReport#228
wsodsong merged 3 commits into
mainfrom
ws/fix-flacky-testarchiveinquirer

Conversation

@wsodsong

Copy link
Copy Markdown
Contributor

Description

The test used a 1s ticker with a 1s sleep, creating a race between the ticker and the finished signal. This change reduces the ticker to 500ms so the ticker is guarantee to fire before shutdown.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (changes that do NOT affect functionality)
  • Adds or updates testing
  • This change requires a documentation update

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adjusts the TestArchiveInquirer_RunProgressReport timing to reduce flakiness caused by a race between the ticker firing and the shutdown signal in the archive inquirer progress reporter.

Changes:

  • Reduced the progress-report ticker interval used by the test to 500ms.
  • Increased the test’s wait time before signaling shutdown to 1s to allow a tick/log to occur.
Comments suppressed due to low confidence (1)

executor/extension/statedb/archive_inquirer_test.go:318

  • duration is now only used for the ticker interval (the sleep is hard-coded to 1s). Renaming this variable to something like tickerDuration would make the intent clearer and avoid confusion about what the value controls.
	duration := 500 * time.Millisecond
	inquirer := &archiveInquirer{
		log:            mockLog,
		finished:       utils.MakeEvent(),
		tickerDuration: duration,
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread executor/extension/statedb/archive_inquirer_test.go
@codecov

codecov Bot commented Mar 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wsodsong
wsodsong force-pushed the ws/fix-flacky-testarchiveinquirer branch from 8d0b86c to 5e0f593 Compare March 27, 2026 07:40
@wsodsong
wsodsong requested review from cabrador and matejmode March 27, 2026 09:04
cabrador
cabrador previously approved these changes Mar 27, 2026
Comment thread executor/extension/statedb/archive_inquirer_test.go Outdated
wsodsong added 3 commits June 30, 2026 14:36
The test used a 1s ticker with a 1s sleep, creating a race between
the ticker and the finished signal. This change reduces the ticker
to 500ms so the ticker is guarantee to fire before shutdown.
@wsodsong
wsodsong force-pushed the ws/fix-flacky-testarchiveinquirer branch from 33e97ef to ae908f9 Compare June 30, 2026 07:36
@rpl-ffl
rpl-ffl self-requested a review June 30, 2026 08:07

@rpl-ffl rpl-ffl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ty!

@wsodsong
wsodsong merged commit 1944a0a into main Jun 30, 2026
6 checks passed
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.

4 participants