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

feat: make sequencer public, add option to run tests in random order #1582

Merged
merged 11 commits into from Jul 6, 2022

Conversation

sheremet-va
Copy link
Member

@sheremet-va sheremet-va commented Jul 3, 2022

Closes #1570

TODO

  • Docs
  • Tests

This PR adds a few new options to config:

  1. --shuffle to CLI flags
  2. sequence to options
  • sequencer allows users to overwrite builtin logic for sharding and sorting
  • shuffle allows tests to be run in random order
  • seed makes random order more deterministic

Also added shuffle flag to describe block to make only part of suite random. I've noticed we have some tests rely on running in the same order, so I though this option might be helpful.

@netlify
Copy link

netlify bot commented Jul 3, 2022

Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 8cade60
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/62c55c9bb99f0d0008df8a57
😎 Deploy Preview https://deploy-preview-1582--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@sheremet-va sheremet-va changed the title feat: make sequencer public, add random option feat: make sequencer public, add option to run tests in random order Jul 4, 2022
Vitest provides a way to run all tests in random order via CLI flag [`--random`](/guide/cli) or config option [`sequence.random`](/config/#sequence-random), but if you want to have only part of your test suite to run tests in random order, you can mark it with this flag.

```ts
describe.random('suite', () => {
Copy link
Member

@antfu antfu Jul 4, 2022

Choose a reason for hiding this comment

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

If so, should we also support repeat for describe? For example:

describe.repeat(10).random('repect 10 times randomly', () => {
  // ...
})

Copy link
Member Author

Choose a reason for hiding this comment

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

How is this has anything to do with random? 👀

Copy link
Member

@antfu antfu Jul 5, 2022

Choose a reason for hiding this comment

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

No necessary in this PR, just an idea. Using random feels more unstable to identify the ordering bug (like sometimes the change passes the CI because the order does not hit the bug, but will later). Running random tests multiple times could reduce such scenarios and make the tests more stable.

Copy link
Member Author

Choose a reason for hiding this comment

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

No necessary in this PR, just an idea. Using random feels more unstable to identify the ordering bug (like sometimes the change passes the CI because the order does not hit the bug, but will later). Running random tests multiple times could reduce such scenarios and make the tests more stable.

Sure, but this also should be done on the global level, yes? This PR adds vitest --random, so there should also be vitest --random --repeat 10.

It's also not clear what should be done on error. Do we fail the suite when it hits the first error? Do we show summary of all 10 runs? I think if we want to add this, it's better be done in other PR, and if/when people will ask for it.

I really want to switch to @vitest/browser, but all this enhancements PRs stop me 😄

Copy link
Member

Choose a reason for hiding this comment

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

Sure we could do that later 👍

Copy link
Member

@antfu antfu left a comment

Choose a reason for hiding this comment

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

The feature looks good to me. But do you think shuffle would be better than random?

@sheremet-va
Copy link
Member Author

sheremet-va commented Jul 5, 2022

The feature looks good to me. But do you think shuffle would be better than random?

Well, it's sequence.random, so I think it's straightforward enough, or do you mean --random?

@antfu
Copy link
Member

antfu commented Jul 6, 2022

I mean the term random -> shuffle in general. From my understanding:

  • random is a more general term, it could combine with many things, like random in order, or decide if a test should run randomly, or pick a random test, or both.
  • shuffle specifically means random the order of an array, in which the items of that array are still the same.

Refs:

@sheremet-va
Copy link
Member Author

I mean the term random -> shuffle in general. From my understanding:

  • random is a more general term, it could combine with many things, like random in order, or decide if a test should run randomly, or pick a random test, or both.
  • shuffle specifically means random the order of an array, in which the items of that array are still the same.

Refs:

Sure, let's name it shuffle instead.

@sheremet-va sheremet-va requested a review from antfu July 6, 2022 06:53
@sheremet-va
Copy link
Member Author

@antfu renamed :)

docs/config/index.md Outdated Show resolved Hide resolved
docs/api/index.md Outdated Show resolved Hide resolved
sheremet-va and others added 2 commits July 6, 2022 12:57
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
Co-authored-by: Anthony Fu <anthonyfu117@hotmail.com>
@sheremet-va sheremet-va enabled auto-merge (squash) July 6, 2022 09:59
@sheremet-va sheremet-va merged commit 5d0dfe9 into vitest-dev:main Jul 6, 2022
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.

Option to run tests in random order
2 participants