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

refactorize the whole test suite #3085

Merged
merged 5 commits into from
Nov 23, 2022
Merged

Conversation

irevoire
Copy link
Member

  1. Make a call to assert_internally_consistent automatically when snapshotting the scheduler. There is no point in snapshotting something broken and expecting the dumb humans to notice.
  2. Replace every possible call to assert_internally_consistent with a snapshot of the scheduler. It uses the same amount of lines and ensures we never change something without noticing in any tests ever.
  3. Name every snapshot: it's easier to debug when something goes wrong and easier to review in general.
  4. Stop skipping breakpoints; it's too easy to miss something. Now you must explicitly show which path the scheduler is supposed to use.
  5. Add a timeout on the channel.recv, it eases the process of writing tests; now, when something files, you get a failure instead of a deadlock.

@curquiza
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Nov 22, 2022
@bors
Copy link
Contributor

bors bot commented Nov 22, 2022

try

Build failed:

@curquiza
Copy link
Member

@irevoire the tests fail

loiclec
loiclec previously approved these changes Nov 23, 2022
Copy link
Contributor

@loiclec loiclec left a comment

Choose a reason for hiding this comment

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

Suuuuuper nice! Love it.
I trust that you didn't actually change the behaviour of the tests and that the content of the insta-snapshot files didn't change?
If so, LGTM once the failing test is fixed 👍

@irevoire
Copy link
Member Author

irevoire commented Nov 23, 2022

I trust that you didn't actually change the behaviour of the tests and that the content of the insta-snapshot files didn't change?

Yep, but I also added a lot of new snapshots

1. Make a call to assert_internally_consistent automatically when snapshoting the scheduler. There is no point in snapshoting something broken and expect the dumb humans to notice.
2. Replace every possible call to assert_internally_consistent by a snapshot of the scheduler. It takes as many lines and ensure we never change something without noticing in any tests ever.
3. Name every snapshots: it's easier to debug when something goes wrong and easier to review in general.
4. Stop skipping breakpoints, it's too easy to miss something. Now you must explicitely show which path is the scheduler supposed to use.
5. Add a timeout on the channel.recv, it eases the process of writing tests, now when something file you get a failure instead of a deadlock.
Copy link
Member

@Kerollmops Kerollmops left a comment

Choose a reason for hiding this comment

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

This looks way better than before, we will be able to catch more bugs, and I am happy that the tests are passing right now 😅

bors merge

@irevoire
Copy link
Member Author

bors merge

@irevoire irevoire added the maintenance Issue about maintenance (CI, tests, refacto...) label Nov 23, 2022
@bors
Copy link
Contributor

bors bot commented Nov 23, 2022

@bors bors bot merged commit aaf5abb into release-v0.30.0 Nov 23, 2022
@bors bors bot deleted the refactorize-the-test-suite branch November 23, 2022 20:19
@meili-bot meili-bot added the v0.30.0 PRs/issues solved in v0.30.0 released on 2022-11-28 label Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Issue about maintenance (CI, tests, refacto...) v0.30.0 PRs/issues solved in v0.30.0 released on 2022-11-28
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants