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

ClassOrderer.Random and MethodOrderer.Random do not use same default seed #3817

Closed
kurellajunior opened this issue May 14, 2024 · 3 comments · Fixed by #3821
Closed

ClassOrderer.Random and MethodOrderer.Random do not use same default seed #3817

kurellajunior opened this issue May 14, 2024 · 3 comments · Fixed by #3821

Comments

@kurellajunior
Copy link

kurellajunior commented May 14, 2024

The documentation is very misleading. It states, that the seed can be set individually for classes and methods, but neither configuration key is actually used:

instead for both random seeds is used MethodOrderer.Random#RANDOM_SEED_PROPERTY_NAME

public static final String RANDOM_SEED_PROPERTY_NAME = "junit.jupiter.execution.order.random.seed";

This is also very inconsistent with the output of the tests which clearly suggests to set the respective seed to a specific value.

I do not know what is tha actual desired behaviour, so cannot provide a PR

@marcphilipp
Copy link
Member

The default order can be configured separately for classes and methods using the junit.jupiter.testclass.order.default and junit.jupiter.testmethod.order.default configuration parameters, respectively. Both ClassOrderer and MethodOrderer use the same junit.jupiter.execution.order.random.seed configuration parameter for customizing the seed used with their random orderers, though.

The documentation is very misleading. It states, that the seed can be set individually for classes and methods

Where does it state that?

neither configuration key is actually used

By "both" are you referring to the setting of the default class/method orderer or the custom seed?

This is also very inconsistent with the output of the tests which clearly suggests to set the respective seed to a specific value.

Which tests are you referring to?

@kurellajunior
Copy link
Author

The default order can be configured separately for classes and methods using the junit.jupiter.testclass.order.default and junit.jupiter.testmethod.order.default configuration parameters, respectively. Both ClassOrderer and MethodOrderer use the same junit.jupiter.execution.order.random.seed configuration parameters for customizing the seed used with their random orderers, though.

that is understood

This is also very inconsistent with the output of the tests which clearly suggests setting the respective seed to a specific value.

Which tests are you referring to?

When I set the ClassOrderer$Random and MethodOrderer$Random as the test class and testmethod order default respectively, the tests log on the CONFIG level the (different) random seeds used. The assumption is they do so to replay a test run with the defined seeds. This is impossible, as both classes use the same configuration key as a custom seed

The documentation is very misleading. It states, that the seed can be set individually for classes and methods

Where does it state that?

Indeed now I see, it states here the opposite:

* <p>The same property is used by {@link MethodOrderer.Random} for
* consistency between the two random orderers.

But actually uses two different seeds when running on fallback. This is then the inconsistency

Neither configuration key is actually used

By "both" are you referring to the setting of the default class/method orderer or the custom seed?

At least I could not find any documentation on how to use ClassOrderer#RANDOM_SEED_PROPERTY_NAME or MethodOrderer#DEFAULT_ORDER_PROPERTY_NAME. Luckily GitHub Copilot knew how to set those in the properties…

@sbrannen sbrannen added this to the 5.11 M2 milestone May 15, 2024
@sbrannen sbrannen changed the title ClassOrderer.Random.RANDOM_SEED_PROPERTY_NAME not used ClassOrderer.Random and MethodOrderer.Random do not use same default seed May 15, 2024
@sbrannen
Copy link
Member

Thanks for the explanation, @kurellajunior. 👍

But actually uses two different seeds when running on fallback. This is then the inconsistency

That's an actual bug.

I've changed the title of this issue to reflect that, and I've assigned this to the 5.11 M2 milestone.

@marcphilipp marcphilipp self-assigned this May 17, 2024
marcphilipp added a commit that referenced this issue May 17, 2024
Prior to this commit the default seeds were generated separately but the
configuration parameter that allows using a fixed seed only allowed to
set both to the same value making it impossible to reproduce a failure
for different default seeds. Since the default seed is now identical,
this scenario is avoided.

Fixes #3817.
marcphilipp added a commit that referenced this issue May 17, 2024
Prior to this commit the default seeds were generated separately but the
configuration parameter that allows using a fixed seed only allowed to
set both to the same value making it impossible to reproduce a failure
for different default seeds. Since the default seed is now identical,
this scenario is avoided.

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

Successfully merging a pull request may close this issue.

3 participants