Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mock-consensus: ⭐ assert that TestNode<C> is Send + Sync #4470

Merged
merged 1 commit into from
May 28, 2024

Conversation

cratelyn
Copy link
Contributor

while refactoring the test node, i found that this is an assumption that isn't explicitly upheld, but is often expected by test code.

to prevent future changes/additions from breaking these properties, add some compile-time assertions that a TestNode<()> is both Send and Sync.

✅ checklist before requesting a review

  • if this code contains consensus-breaking changes, i have added the
    "consensus-breaking" label. otherwise, i declare my belief that there are not
    consensus-breaking changes, for the following reason:

    this only makes compile-time assertions on test code.

while refactoring the test node, i found that this is an assumption that
isn't explicitly upheld, but is often expected by test code.

to prevent future changes/additions from breaking these properties, add
some compile-time assertions that a `TestNode<()>` is both `Send` and
`Sync`.
@cratelyn cratelyn added A-mock-consensus Area: Relates to the mock consensus engine C-refactor Category: refactors and other related improvements labels May 24, 2024
@cratelyn cratelyn added this to the Sprint 7 milestone May 24, 2024
@cratelyn cratelyn self-assigned this May 24, 2024
@cratelyn cratelyn added the _P-low Priority: low label May 24, 2024
@cratelyn cratelyn marked this pull request as ready for review May 24, 2024 20:05
@cratelyn cratelyn merged commit 28bcd88 into main May 28, 2024
13 checks passed
@cratelyn cratelyn deleted the kate/test-node-is-send-and-sync branch May 28, 2024 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mock-consensus Area: Relates to the mock consensus engine C-refactor Category: refactors and other related improvements _P-low Priority: low
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants