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

Restructure integration tests to make rust-analyzer happy #4510

Merged
merged 7 commits into from
Apr 10, 2023
Merged

Conversation

lutter
Copy link
Collaborator

@lutter lutter commented Apr 1, 2023

This PR removes the cyclic dev-dependencies that make rust-analyzer almost useless by moving integration tests from their individual crates into the test-store crate. It's a little weird to have the tests outside their crates, but almost all integration tests really do need the whole system directly or indirectly.

Given a choice between a little weirdness with test placement and a rust-analyzer that by now is basically unable to find anything, I'd rather live with the weirdness.

- check that the writer thread is still running when accepting items to be
  queued. It makes no sense to accept requests when the writer thread is
  not running, and should therefore fail early.
- change the mechanism for how tests can single-step a deployment to be
  specific to that deployment to avoid problems with tests running at the
  same time. This was a huge footgun, and getting caught in that looks
  like a deadlock
- implement Debug for queued requests
Copy link
Collaborator

@leoyvens leoyvens left a comment

Choose a reason for hiding this comment

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

Great quality of life improvement for all of us!

I guess store: Several improvements to Writable is mostly unrelated, but that looks good as well.

@lutter
Copy link
Collaborator Author

lutter commented Apr 10, 2023

I guess store: Several improvements to Writable is mostly unrelated, but that looks good as well.

That one was actually necessary (and the hardest part of this PR) since all the store/postgres tests now run as one test, and the single-step logic caused tests to deadlock.

@lutter lutter merged commit 3ba05c3 into master Apr 10, 2023
@lutter lutter deleted the lutter/tests branch April 10, 2023 19:15
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