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

Run tests in random rather than alphabetical order #21467

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lmartelli
Copy link

@lmartelli lmartelli commented Mar 17, 2023

Running tests in alphabetical order is a bad practice : it can hide unwanted dependencies between tests. Therefore I believe it is preferable to run them in random order so as that to spot problems as soon as possible. If some tests are flaky because of unwanted dependencies are incomplete cleanup, it's best to fix them rather than hide the problem and look away 🙈
And if you really need a specific order for some tests, you can still use @TestMethodOrder.


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

@mshima
Copy link
Member

mshima commented Mar 18, 2023

I agree that would be best with random order.
But I think we should keep ordered at our CI due to complexity of JHipster.
A random failure is not reproducible. And:

  • most contributors cannot merge PRs with failures.
  • can break workflows like dependabot.

Random failures is really bad for JHipster CI.

@mshima
Copy link
Member

mshima commented Mar 22, 2023

If a seed based on baseName is added https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#runOrderRandomSeed. I think it's acceptable.
Like this:

const faker = await createFaker();
faker.seed(stringHashCode(application.baseName));
application.keycloakSecrets = Array.from(Array(6), () => faker.datatype.uuid());

@heilaschaefer
Copy link

Random order is interesting. However if only one set of orders causes a failure, then it would be necessary to be able to reproduce that set of orders. I suggest that a Seed for the random is clearly published so that it makes the order reproducible if need be.

@mraible
Copy link
Contributor

mraible commented Jun 20, 2024

Closing and reopening to restart CI.

@mraible mraible closed this Jun 20, 2024
@mraible mraible reopened this Jun 20, 2024
@mshima
Copy link
Member

mshima commented Jun 21, 2024

I don’t think we should merge without adding a reproducible seed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants