Skip to content

fix a BEAUti bug to drop package-provided InputEditors and add tests - #154

Merged
jordandouglas merged 1 commit into
masterfrom
pkginputeditor
Aug 20, 2026
Merged

fix a BEAUti bug to drop package-provided InputEditors and add tests #154
jordandouglas merged 1 commit into
masterfrom
pkginputeditor

Conversation

@walterxie

Copy link
Copy Markdown
Member

Fix #153

Problem

InputEditorFactory.init() only consulted PackageManager.listServices() (the
mechanism that discovers InputEditors contributed by external BEAST packages, via
their version.xml) when ServiceLoader.load() had found zero editors. Since a
real BEAUti run always finds several core beast.fx editors via ServiceLoader first,
that condition was never true in practice — meaning package-provided editors (e.g.
starbeast3's MRCAPriorInputEditorSB3) were never registered, silently. See #.

ServiceLoader.load() cannot see into the separate JPMS ModuleLayer that
PackageManager loads each package into at runtime — that's expected JPMS design, not
a defect — so PackageManager.listServices() is not optional supplementary discovery;
it's the only path to package-provided editors and must always run.

Fix

beast-fx/src/main/java/beastfx/app/inputeditor/InputEditorFactory.java:
removed the inputEditorMap.size() == 0 (and the now-redundant isJUnitTest()) guard
around the PackageManager.listServices() call, so it always runs in addition to the
ServiceLoader.load() pass, merging results from both. registerInputEditors()
already just adds map entries, so this cannot clobber any core editor already found by
ServiceLoader — it can only add entries for types core editors don't already cover.

- if (beast.pkgmgmt.Utils6.isJUnitTest() || inputEditorMap.size() == 0) {
-     Set<String> inputEditors = PackageManager.listServices("beastfx.app.inputeditor.InputEditor");
-     registerInputEditors(inputEditors.toArray(new String[0]));
- }
+ Set<String> inputEditors = PackageManager.listServices("beastfx.app.inputeditor.InputEditor");
+ registerInputEditors(inputEditors.toArray(new String[0]));

Testing

Added InputEditorFactoryPackageDiscoveryTest (beast-fx/src/test/java/test/beastfx/app/beauti/),
a regression test that reproduces the bug without needing a real external package or a
second ModuleLayer:

- beastfx.app.inputeditor.BEASTObjectInputEditor (a real beast.fx core editor) stands
  in for "a core editor ServiceLoader finds first." A test-only
  META-INF/services/beastfx.app.inputeditor.InputEditor resource makes it
  discoverable under a plain classpath test run (e.g. Maven Surefire); under a true
  JPMS module-path run (e.g. IntelliJ's default test runner) it's found via its own
  genuine provides declaration instead — either way, inputEditorMap is non-empty
  going into pass 2, matching real BEAUti.
- test.beastfx.app.beauti.fixtures.PackageProvidedInputEditor stands in for an
  external package's editor: registered only via a test version.xml <service>
  entry, invisible to ServiceLoader.load(). Whether this one gets registered is the
  actual regression check.
- The registration call runs via Platform.runLater rather than directly on the JUnit
  thread, since the pre-fix guard's || isJUnitTest() clause would otherwise mask the
  bug under test.

Verified both directions by toggling the fix:
- Unpatched InputEditorFactory: test fails (PackageProvidedInputEditor not found).
- Patched: test passes.
- Full beast-fx suite: 37/37 passing.
- Confirmed against real starbeast3 too: built it as an installable package, loaded it
  into its own plugin ModuleLayer via PackageManager (the real production path),
  and confirmed MRCAPriorSB3.class now resolves to
  starbeast3.app.beauti.MRCAPriorInputEditorSB3 in InputEditorFactory's registry
  (previously resolved to null / fell back to the generic superclass editor).

Files changed

- beast-fx/src/main/java/beastfx/app/inputeditor/InputEditorFactory.java — the fix
- beast-fx/src/test/java/test/beastfx/app/beauti/InputEditorFactoryPackageDiscoveryTest.java — regression test
- beast-fx/src/test/java/test/beastfx/app/beauti/fixtures/PackageProvidedInputEditor.java — test fixture
- beast-fx/src/test/resources/META-INF/services/beastfx.app.inputeditor.InputEditor — test fixture registration
- beast-fx/src/test/resources/version.xml — test fixture registration

@jordandouglas
jordandouglas merged commit 9dfb20a into master Aug 20, 2026
1 check passed
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.

BEAUti silently drops package-provided InputEditors when core editors are found first

2 participants