Menu
▾
▴
[Pcgen-github] [PCGen/pcgen] eeebb3: TestHelper: load plugins via JUnit extension; clea...
|
From: Vest <no...@gi...> - 2026-06-04 05:40:23
|
Branch: refs/heads/master Home: https://github.com/PCGen/pcgen Commit: eeebb35f35190d01011f0d19561d227265a925dc https://github.com/PCGen/pcgen/commit/eeebb35f35190d01011f0d19561d227265a925dc Author: Vest <Ve...@us...> Date: 2026-06-04 (Thu, 04 Jun 2026) Changed paths: M .github/workflows/gradle-release-manual.yml M .github/workflows/gradle-release.yml M .github/workflows/gradle-test.yml M PCGen-Formula/build.gradle M PCGen-Formula/code/src/test/pcgen/base/testsupport/AbstractFormulaTestCase.java M PCGen-Formula/code/src/test/pcgen/base/testsupport/TestUtilities.java M build.gradle M code/src/itest/pcgen/cdom/facet/StatIntegrationTest.java M code/src/itest/pcgen/io/RaceTargetSaveRestoreTest.java M code/src/itest/plugin/lsttokens/datacontrol/SelectableTokenIntegrationTest.java M code/src/itest/plugin/lsttokens/editcontext/testsupport/AbstractBigDecimalIntegrationTestCase.java M code/src/itest/plugin/lsttokens/editcontext/testsupport/AbstractIntegerIntegrationTestCase.java M code/src/itest/tokencontent/GlobalModifyTest.java M code/src/java/pcgen/system/Main.java M code/src/test/pcgen/AbstractCharacterTestCase.java M code/src/test/pcgen/AbstractJunit5CharacterTestCase.java M code/src/test/pcgen/cdom/helper/AspectTest.java M code/src/test/pcgen/core/BonusManagerTest.java M code/src/test/pcgen/core/ChallengeRatingPathfinderTest.java M code/src/test/pcgen/core/EquipmentModifierTest.java M code/src/test/pcgen/core/PCClassTest.java M code/src/test/pcgen/core/PCTemplateTest.java M code/src/test/pcgen/core/PObjectTest.java M code/src/test/pcgen/core/PlayerCharacterSpellTest.java M code/src/test/pcgen/core/PrereqHandlerTest.java M code/src/test/pcgen/core/StatListTest.java M code/src/test/pcgen/core/analysis/SpellLevelTest.java M code/src/test/pcgen/core/levelability/AddClassSkillsTest.java M code/src/test/pcgen/core/prereq/PreAbilityTest.java M code/src/test/pcgen/core/prereq/PreAlignTest.java M code/src/test/pcgen/core/prereq/PreArmorProfTest.java M code/src/test/pcgen/core/prereq/PreArmorTypeTest.java M code/src/test/pcgen/core/prereq/PreAttTest.java M code/src/test/pcgen/core/prereq/PreBirthplaceTest.java M code/src/test/pcgen/core/prereq/PreCSkillTest.java M code/src/test/pcgen/core/prereq/PreCampaignTest.java M code/src/test/pcgen/core/prereq/PreCheckBaseTest.java M code/src/test/pcgen/core/prereq/PreCheckTest.java M code/src/test/pcgen/core/prereq/PreClassTest.java M code/src/test/pcgen/core/prereq/PreDRTest.java M code/src/test/pcgen/core/prereq/PreDeityAlignTest.java M code/src/test/pcgen/core/prereq/PreDeityDomainTest.java M code/src/test/pcgen/core/prereq/PreDeityTest.java M code/src/test/pcgen/core/prereq/PreDomainTest.java M code/src/test/pcgen/core/prereq/PreEquipPrimaryTest.java M code/src/test/pcgen/core/prereq/PreEquipSecondaryTest.java M code/src/test/pcgen/core/prereq/PreEquipTest.java M code/src/test/pcgen/core/prereq/PreEquipTwoWeaponTest.java M code/src/test/pcgen/core/prereq/PreGenderTest.java M code/src/test/pcgen/core/prereq/PreHDTest.java M code/src/test/pcgen/core/prereq/PreHPTest.java M code/src/test/pcgen/core/prereq/PreHandsTest.java M code/src/test/pcgen/core/prereq/PreItemTest.java M code/src/test/pcgen/core/prereq/PreKitTest.java M code/src/test/pcgen/core/prereq/PreLangTest.java M code/src/test/pcgen/core/prereq/PreLegsTest.java M code/src/test/pcgen/core/prereq/PreLevelMaxTest.java M code/src/test/pcgen/core/prereq/PrePCLevelTest.java M code/src/test/pcgen/core/prereq/PreRaceTest.java M code/src/test/pcgen/core/prereq/PreReqHandlerTest.java M code/src/test/pcgen/core/prereq/PreRuleTest.java M code/src/test/pcgen/core/prereq/PreShieldProfTest.java M code/src/test/pcgen/core/prereq/PreSizeTest.java M code/src/test/pcgen/core/prereq/PreSpellDescriptorTest.java M code/src/test/pcgen/core/prereq/PreSpellSubSchoolTest.java M code/src/test/pcgen/core/prereq/PreStatTest.java M code/src/test/pcgen/core/prereq/PreSubClassTest.java M code/src/test/pcgen/core/prereq/PreTemplateTest.java M code/src/test/pcgen/core/prereq/PreTypeTest.java M code/src/test/pcgen/core/prereq/PreVarTest.java M code/src/test/pcgen/core/term/EvaluatorFactoryTest.java M code/src/test/pcgen/core/term/PCRacialHDSizeTermEvaluatorTest.java M code/src/test/pcgen/gui2/facade/CharacterAbilitiesTest.java M code/src/test/pcgen/gui2/facade/CharacterFacadeImplTest.java M code/src/test/pcgen/gui2/facade/CompanionSupportFacadeImplTest.java M code/src/test/pcgen/gui2/facade/EquipmentSetFacadeImplTest.java M code/src/test/pcgen/gui2/facade/Gui2InfoFactoryTest.java M code/src/test/pcgen/gui2/facade/SpellBuilderFacadeImplTest.java M code/src/test/pcgen/io/PCGVer2ParserCharacterTest.java M code/src/test/pcgen/io/exporttoken/AbilityListTokenTest.java M code/src/test/pcgen/io/exporttoken/SpellMemTokenTest.java M code/src/test/pcgen/io/exporttoken/TextTokenTest.java M code/src/test/pcgen/io/exporttoken/VarTokenTest.java M code/src/test/pcgen/persistence/lst/DataLoadTest.java M code/src/test/pcgen/persistence/lst/DataTest.java M code/src/test/pcgen/persistence/lst/FeatTest.java M code/src/test/pcgen/persistence/lst/InstallLoaderTest.java M code/src/test/pcgen/persistence/lst/MigrationLoaderTest.java M code/src/test/pcgen/persistence/lst/PObjectLoaderTest.java M code/src/test/pcgen/persistence/lst/output/prereq/PrerequisiteWriterTest.java M code/src/test/pcgen/persistence/lst/prereq/PreCheckParserTest.java M code/src/test/pcgen/persistence/lst/prereq/PreEquipTest.java M code/src/test/pcgen/persistence/lst/prereq/PreItemTest.java M code/src/test/pcgen/persistence/lst/prereq/PreKitParserTest.java M code/src/test/pcgen/persistence/lst/prereq/PreLanguageParserTest.java M code/src/test/pcgen/persistence/lst/prereq/PreMoveParserTest.java M code/src/test/pcgen/persistence/lst/prereq/PreMultParserTest.java M code/src/test/pcgen/persistence/lst/prereq/PreParserFactoryTest.java M code/src/test/pcgen/persistence/lst/prereq/PreStatParserTest.java M code/src/test/pcgen/util/TestHelper.java M code/src/test/plugin/exporttokens/AttackTokenTest.java M code/src/test/plugin/exporttokens/CampaignHistoryTokenTest.java M code/src/test/plugin/exporttokens/SkillSitTokenTest.java M code/src/test/plugin/exporttokens/SkillTokenTest.java M code/src/test/plugin/exporttokens/SpellListTokenTest.java M code/src/test/plugin/jepcommands/CountCommandTest.java M code/src/test/plugin/jepcommands/CountDistinctCommandTest.java A code/src/testcommon/pcgen/test/PCGenTestEnvironment.java M code/src/utest/pcgen/cdom/base/FormulaFactoryTest.java M code/src/utest/pcgen/persistence/RecursiveFileFinderTest.java M code/src/utest/plugin/lsttokens/FollowersLstTest.java M code/src/utest/plugin/lsttokens/VisionLstTest.java Log Message: ----------- TestHelper: load plugins via JUnit extension; cleanup (#7580) * TestHelper: load plugins via static initializer `TestHelper.loadPlugins()` was already idempotent — a `boolean loaded` flag guarded the expensive `Main.createLoadPluginTask().run()` call so only the first invocation did real work. The remaining cleanup turns that runtime guard into a structural one: * Replace the mutable `boolean loaded` field + `if (!loaded)` branch with a private static initializer. Plugin loading now runs exactly once when `TestHelper` is first class-loaded, enforced by the JVM rather than by a flag check that future maintainers could bypass. * Drop the two internal `loadPlugins()` calls in `makeEquipment` and `makeAbilityFromString` — they're inside the same class that defines the no-op, so by the time the JVM is executing those methods the static initializer has already run. * Keep the no-op `loadPlugins()` method and the 12 external call sites. In most callers `loadPlugins()` is the only `TestHelper` reference, so removing it would let unused-import cleanup strip `import TestHelper`, which would in turn prevent class-loading and skip the static initializer. The "redundant" calls are load-bearing: they keep `TestHelper` on each test's class-load graph. * Add an inline comment to silence SonarQube S1186 ("Methods should not be empty") and to record the load-bearing rationale in-place. * TestHelper: clean up IDE-flagged warnings - Remove unused imports (BufferedReader, FileInputStream, InputStreamReader, PersistenceLayerException) - Drop unthrown 'throws PersistenceLayerException' from parsePCClassText - Parameterize Class<?> in createSource - Replace string-based "Object".equals(clazz.getName()) walk-stop with identity compare clazz != Object.class - Inline redundant pccFilesPath local in findDataFolder - Switch three concatenated LOG.info calls to parameterized LOG.log - Fill in missing @param entries on hasWeaponProfKeyed * Replace local parsePCClassText copies with TestHelper.parsePCClassText PCClassTest and AddClassSkillsTest each carried a private duplicate of the same parsePCClassText helper, declared with an unthrown PersistenceLayerException. Now that the canonical helper in TestHelper has had its stale throws removed, the local copies are pure dead code and the AddClassSkillsTest try/catch was already unreachable. * TestHelper: drop dead methods makeAbilityFromString, makeWeaponProf, loadGameModes None of the three are referenced anywhere in the repo (no callers, no reflective lookups). Removing them also lets us drop the unused abLoader and source fields plus six imports they pulled in (AbilityLoader, CampaignFileLoader, ConfigurationSettings, PCGenTask, PropertyContextFactory, SystemUtils). * ci: add --stacktrace --info to test runs to debug datatest crash * TestFX: use --add-opens for ParametersImpl on JavaFX 25 TestFX's ApplicationExtension.afterEach calls cleanupParameters, which does setAccessible(true) on the private static ParametersImpl.params field — deep reflection that --add-exports doesn't grant. Pre-Java 25 this slipped through; on JavaFX 25 it throws InaccessibleObjectException ("module javafx.graphics does not opens com.sun.javafx.application to unnamed module") and crashes the test JVM, which surfaces downstream as "non-zero exit value 1" with no test failures reported. Replace --add-exports with --add-opens for that single package; the rest of the JVM-arg block is correct. Also revert the temporary --stacktrace --info debug flags from the test workflow (they were a one-shot diagnostic, not a permanent setting; they balloon the CI log to ~100k lines). * ci: temporarily add --stacktrace --info to expose datatest startup crash * ci: revert temporary --stacktrace --info — diagnosis done * Main: extract runBootstrapTasks() to capture canonical init order The plugin/game-mode/campaign loader sequence was duplicated verbatim in startupWithGUI() and startupWithoutGUI(). Pull it into a single package-private helper so test code can run the same sequence and the order can never drift between the two callers. Behaviour-preserving refactor; no production changes. * Add PCGenTestEnvironment JUnit 5 extension Loads PCGen plugins exactly once per JVM via a BeforeAllCallback, keyed on ExtensionContext.Store.GLOBAL so the work happens at most once across the whole test JVM. Auto-discovered via META-INF/services and the autodetection.enabled property, so test classes don't need any annotation to opt in. This is the proper replacement for the static initializer the previous commit introduced: BeforeAllCallback fires after JUnit's lifecycle is set up but before any user @BeforeAll runs, so it no longer perturbs the init order that DataTest/DataLoadTest require. * TestHelper: remove static initializer; mark loadPlugins() deprecated The static initializer ran Main.createLoadPluginTask() at class-load time — earlier than DataTest/DataLoadTest's expected init order, which caused Main.loadProperties() to find no settings file and call GracefulExit.exit(1), tearing down the test JVM with no stack trace. PCGenTestEnvironment (added in the previous commit) now owns plugin loading, runs at the correct point in the JUnit lifecycle, and is auto-discovered for every test class. Keep loadPlugins() as a deprecated no-op so existing call sites keep compiling — they can be cleaned up incrementally rather than all in one go. * DataTest/DataLoadTest: use Main.runBootstrapTasks() for init order The two loadGameModes() helpers each open-coded the same 4-step sequence (createLoadPluginTask + GameModeFileLoader + CampaignFileLoader, each constructed and run individually). Replace with the runBootstrapTasks() helper extracted in the earlier commit, so all three call sites (production GUI, production headless, test) share one definition of the canonical bootstrap order. Make runBootstrapTasks public — it's the same visibility the existing loadProperties() and createLoadPluginTask() helpers already have. * PCGenTestEnvironment: opt-in via @ExtendWith, not auto-discovery Auto-discovery via META-INF/services would force plugin loading on EVERY test class in the JVM, including ones that explicitly assume plugins are NOT loaded. Concrete example: SetSolverManagerTest.setUp constructs a fresh VariableContext and calls addFunction(getOther), which fails with 'Cannot load two functions of name: getOther' if PluginFunctionLibrary already has it (plugin loading populates that singleton, and VariableContext's constructor copies all entries from it). Make plugin loading opt-in: @ExtendWith(PCGenTestEnvironment.class) on classes that need it. Apply to AbstractCharacterTestCase and AbstractJunit5CharacterTestCase (lifts ~114 subclasses) plus the 10 standalone test classes that previously called loadPlugins() explicitly. Drop the META-INF/services + junit-platform.properties files. * TestHelper: delete deprecated loadPlugins() no-op No call sites remain in the tree. The previous commit kept it as a deprecated shim only to avoid touching every test class in one go; that's now done. Drop it. * ci: enable --stacktrace on every gradlew invocation Costs nothing on green builds (only fires on failure), saves a diagnostic round-trip when CI fails for a non-obvious reason. This session needed two separate diagnostic commits (one for the TestFX/JavaFX 25 InaccessibleObjectException and one for the TestHelper static-initializer-induced JVM exit) precisely because neither failure showed a stack trace by default — Gradle just said 'non-zero exit value 1' and pointed at --stacktrace as the next step. Make that the next step happen automatically. * JUnit 6 cleanup: replace deprecated APIs in test scaffolding ExtensionContext.Store#getOrComputeIfAbsent was deprecated in JUnit 6.0 in favor of #computeIfAbsent (mirrors java.util.Map). Three other files still imported junit.framework.TestCase / org.junit.Test from JUnit 3/4 even though the rest of their bodies were already on Jupiter. PCGenTestEnvironment switches to computeIfAbsent. AbstractFormulaTestCase and TestUtilities drop junit.framework.TestCase.fail in favor of Jupiter Assertions.fail. RecursiveFileFinderTest swaps org.junit.Test for the Jupiter @Test. * JUnit 6 migration: rewrite org.junit.Assert to Jupiter Assertions JUnit 6 drops junit:junit from the classpath, so the org.junit.Assert static helpers used by 86 test files no longer resolve. Each file now imports the equivalent from org.junit.jupiter.api.Assertions. Jupiter reverses the message argument convention: JUnit 4's assertEquals(message, expected, actual) becomes assertEquals(expected, actual, message). Mass-applying the import swap alone would silently match Jupiter's assertEquals(Object, Object, String) overload with `message` read as `expected` and vice versa - the calls would compile but every failure message would be wrong. The 1054 swap-able call sites (assertEquals/assertTrue/assertFalse/assertNull/ assertNotNull/assertSame/assertArrayEquals with leading String literal) are reordered to match Jupiter's convention. Nine sites where the leading message was a String concatenation expression rather than a literal were caught by the compiler and fixed by hand. * Drop JUnit 4 / vintage engine from test classpath With every test file now on Jupiter, neither the junit:junit jar nor the junit-vintage-engine runner is needed. The stale comment claiming "~870 legacy tests" is removed - the actual count was zero by the time the migration finished. To unsubscribe from these emails, change your notification settings at https://github.com/PCGen/pcgen/settings/notifications |