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

fix(ci): pin maven-surefire-plugin to 3.1.2 #1276

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

TTtie
Copy link
Contributor

@TTtie TTtie commented Nov 14, 2023

This PR pins maven-surefire-plugin to version 3.1.2 to resolve build failures for local builds due to resolution algorithm changes in Maven 3.9.x.

On GitHub Actions (and my server running Maven 3.8.7 from Debian repositories), maven-surefire-plugin resolves to version 2.12.4 – which makes the build pass, but also seems to skip a significant portion of defined tests1.

However, when building PGM locally (using Maven 3.9.1 from Fedora's repositories), maven-surefire-plugin is resolved to version 3.0.0 (or higher), which runs all of the defined tests. Building with Maven 3.9.5 on CI runs all defined tests, skipping the TextException class test that is otherwise failing, likely due to the JVM running using a different locale2. This makes PGM build successfully on CI under newer Maven versions.

This PR also fixes a broken test of the TextException class which is crashing due to a missing Bukkit instance when tc.oc.pgm.util.text.Audience is accessed by getting the empty Audience directly. Along with this, the test for localized messages has been improved to be environment agnostic by translating the error manually using the same steps as TextException does and comparing the results appropriately.

Footnotes

  1. https://github.com/PGMDev/PGM/actions/runs/6867090484/job/18674723190#step:4:151

  2. https://github.com/TTtie/PGM/actions/runs/6961974625/job/18944710722#step:6:708

@TTtie TTtie force-pushed the fix/failing-textexception-tests branch from 7b5e830 to fe75917 Compare November 22, 2023 19:48
Pinning this is needed to resolve discrepancies between GHA and local builds (GHA resolves to 2.12.4 - which does not run all tests [1], local clones across my fleet resolve to 3.0.0 - which runs all tests present, one of which is failing, but that will be resolved in a later commit)

[1]: https://github.com/PGMDev/PGM/actions/runs/6867090484/job/18674723190#step:4:151

Signed-off-by: TTtie <[email protected]>
Using tc.oc.pgm.util.text.Audience requires a running Bukkit instance. Accessing it from a test (where a Bukkit instance is N/A) therefore crashes the test, making it fail.

Signed-off-by: TTtie <[email protected]>
Instead of checking for whether the JVM is in English and then checking for a static string (which probably changed in the meantime), reconstruct the translated error manually and compare the results. This makes the test environment agnostic.

Signed-off-by: TTtie <[email protected]>
@TTtie TTtie force-pushed the fix/failing-textexception-tests branch from fe75917 to 5d32ef5 Compare November 23, 2023 23:37
@Electroid Electroid merged commit 6ef8f69 into PGMDev:dev Dec 5, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants