Skip to content

Issue #16233: test execution is failing on non EN locales#16500

Merged
romani merged 1 commit into
checkstyle:masterfrom
AmitKumarDeoghoria:g1
Mar 8, 2025
Merged

Issue #16233: test execution is failing on non EN locales#16500
romani merged 1 commit into
checkstyle:masterfrom
AmitKumarDeoghoria:g1

Conversation

@AmitKumarDeoghoria

@AmitKumarDeoghoria AmitKumarDeoghoria commented Mar 8, 2025

Copy link
Copy Markdown
Contributor

Solves : #16233
Reverted #16234 to verify

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Items


@AfterClass
public static void restoreLocale() {
Locale.setDefault(defaultLocale);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this required? To restore original locale .

Our test needs English locale to let messages in Input files to work properly.

@AmitKumarDeoghoria AmitKumarDeoghoria Mar 8, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hey @romani , I added the code to restore the original locale to maintain test isolation and prevent any side effects on other test cases that might rely on the system’s default locale. But if we are always using English and there are no dependencies on the system locale now or in the future, we can remove it. So should I remove it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We use to be locale based .
But after we migrated to Input based tests with messages inside, it becomes impossible, so we stick to EN locale.

Ok , let's keep it, it should not break anything, this should easier to help identify locale problems in other tests.

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok to merge thanks a lot

@romani romani merged commit 610ba3a into checkstyle:master Mar 8, 2025
mureinik pushed a commit to mureinik/checkstyle that referenced this pull request Oct 6, 2025
…otations

Follow up on checkstyle#16500 - since PackageObjectFactoryTest is executed using
the JUnit Jupiter teset executor, the `org.junit.BeforeClass` and
`org.junit.AfterClass` JUnit 4 annotations aren't picked up, and the
methods annotated with them are never executed.

This patch follows up on the work done in checkstyle#16233 and replaces these
annotations with `org.junit.jupiter.api.BeforeAll` and
`org.junit.jupiter.api.AfterAll`, respectively, so that these methods
are indeed executed as part of the test lifecycle.
mureinik added a commit to mureinik/checkstyle that referenced this pull request Oct 6, 2025
…otations

Follow up on checkstyle#16500 - since PackageObjectFactoryTest is executed using
the JUnit Jupiter teset executor, the `org.junit.BeforeClass` and
`org.junit.AfterClass` JUnit 4 annotations aren't picked up, and the
methods annotated with them are never executed.

This patch follows up on the work done in checkstyle#16233 and replaces these
annotations with `org.junit.jupiter.api.BeforeAll` and
`org.junit.jupiter.api.AfterAll`, respectively, so that these methods
are indeed executed as part of the test lifecycle.
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