Skip to content

tests - ordering of the queue #2072

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

Merged
merged 2 commits into from
Apr 16, 2025

Conversation

Jamesbarford
Copy link
Contributor

  • More tests for the queue invariants. Adding them as guardrails and as cases we can re-implement to ensure the new mechanism behaves in the same way.
  • I also renamed a few things for clarity

@Jamesbarford Jamesbarford force-pushed the tests/tests-for-queue-ordering branch from 56cce4e to f62a596 Compare April 16, 2025 09:56
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

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

Looks great, having more tests is always super useful. Also adding comments to the code that explain what it does, because the logic is quite complicated. Let me know if you want to modify something else, otherwise LGTM.

@Jamesbarford
Copy link
Contributor Author

Looks great, having more tests is always super useful. Also adding comments to the code that explain what it does, because the logic is quite complicated. Let me know if you want to modify something else, otherwise LGTM.

I think for the time being I think this is enough. Presently I'm thinking that the logic from the function calculate_missing_from could be repurposed to insert unsorted data into a possible commit_queue table. I tired splitting the sort to a different step but it bloated out the PR and arguably made the logic harder to follow so I left the flow as it is and changed some variable names.

@Kobzol Kobzol merged commit a86adc1 into rust-lang:master Apr 16, 2025
11 checks passed
@Jamesbarford Jamesbarford deleted the tests/tests-for-queue-ordering branch April 16, 2025 13:19
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