Skip to content

test: extend v19 spork sync wait#7327

Closed
thepastaclaw wants to merge 1 commit into
dashpay:v23.1.xfrom
thepastaclaw:test/v23.1.x-spork-sync-timeout
Closed

test: extend v19 spork sync wait#7327
thepastaclaw wants to merge 1 commit into
dashpay:v23.1.xfrom
thepastaclaw:test/v23.1.x-spork-sync-timeout

Conversation

@thepastaclaw
Copy link
Copy Markdown

Test v19 spork sync wait

Summary

  • Backport the feature_dip3_v19.py spork sync timeout deflake to v23.1.x.
  • Keep the existing activate_by_name() default timeout for other callers.
  • Let the v19 activation test opt into a 60s spork-sync wait.

Context

dashpay/dash#7321 hit the known feature_dip3_v19.py UBSAN timeout while
waiting for sporks to match after v19 activation. This mirrors the green fix
already opened for develop in dashpay/dash#7325.

Validation

  • python3 -m py_compile test/functional/feature_dip3_v19.py test/functional/test_framework/test_framework.py
  • Code review gate: Recommendation: ship

@thepastaclaw
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw
Copy link
Copy Markdown
Author

thepastaclaw commented May 18, 2026

✅ Review complete (commit fb83266)

@github-actions
Copy link
Copy Markdown

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 930f2439-b717-4d31-9c2e-b5e1efe6f812

📥 Commits

Reviewing files that changed from the base of the PR and between b564a33 and fb83266.

📒 Files selected for processing (2)
  • test/functional/feature_dip3_v19.py
  • test/functional/test_framework/test_framework.py

Walkthrough

This PR adds a configurable timeout parameter to the test framework's spork synchronization behavior. The DashTestFramework.activate_by_name() method now accepts an optional spork_sync_timeout parameter (defaulting to 30 seconds) that controls how long the framework waits for sporks to reach a consistent state during both the activation prep phase and the post-activation reversion of SPORK_17_QUORUM_DKG_ENABLED. A test case for v19 activation demonstrates the feature by requesting a 60-second timeout.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: extending the v19 spork sync wait timeout in tests.
Description check ✅ Passed The description is directly related to the changeset, explaining the purpose of adding spork_sync_timeout parameter and its usage in v19 activation test.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

Copy link
Copy Markdown
Author

@thepastaclaw thepastaclaw left a comment

Choose a reason for hiding this comment

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

Code Review

I verified the checked-out worktree at fb83266 and confirmed this PR only changes two functional-test files. The new spork_sync_timeout parameter is appended to DashTestFramework.activate_by_name() with the existing 30-second default preserved for all current callers, and feature_dip3_v19.py is the only caller that opts into a longer 60-second wait. I did not find any correctness, Dash-specific interaction, or test-coverage issue to report.

Reviewed commit: fb83266

@thepastaclaw
Copy link
Copy Markdown
Author

Closing during authored-PR cleanup. This v23.1.x spork-timeout draft duplicates the same flaky-test cleanup direction and should not remain open without release-owner demand.

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