Skip to content

Xmltest#109

Merged
walterxie merged 14 commits into
masterfrom
xmltest
Jun 23, 2026
Merged

Xmltest#109
walterxie merged 14 commits into
masterfrom
xmltest

Conversation

@walterxie

Copy link
Copy Markdown
Member
  1. fix resume bug for testGTR.xml;
  2. prevent silent fail if XML does not exist during the tests;
  3. upgrade tests using old framework junit 4 to junit 5;
  4. test more scenario for resume and add more XMLs to test resume.
  5. move all beastfx resume and XMLParsing tests (test/beastfx/integration/) to base, because they do not depend on beastfx now.

@walterxie walterxie requested review from alexeid and rbouckaert June 22, 2026 23:16

@alexeid alexeid 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.

  1. ParameterUtils.java:53
    The ID-strip fallback branch is effectively dead and throws instead of recovering. The new noboundPattern dropped the old leading ^.*? wildcard and relies on fullStr.startsWith(id) having stripped the ID first. When startsWith(id) is false (e.g. getTextContent() returns " kappa: 29" with leading whitespace from a pretty-printed/hand-edited/foreign state file), str keeps the full text, and the anchored ^(?:{…})?: pattern matches neither. So parseParameter throws "String could not be parsed". The old ^.*? tolerated this. For files written by BEAST3's own paramToString (no leading whitespace) it round-trips fine, so this is a robustness regression on non-canonical input, not a break of the happy path. A fullStr.trim() (or a defined error rather than the silent fallback) would keep the robustness.

  2. XMLProducerTest.java:35
    resolveNexusDir() duplicates XMLPathUtil.resolveExamplesDir(). It re-implements the same getResource → toURI → File + user.dir fallback logic, differing only by appending /nexus. Since XMLProducerTest is in the same package, this could be new File(XMLPathUtil.resolveExamplesDir(), "nexus").getAbsolutePath(). Any future fix to the resolution strategy (jar URLs, fallback path) must now be made in two places.

  3. ResumeTest.java / XMLPathUtil.java:64
    Output isolation rests on the JVM-wide file.name.prefix, making the test order-/fork-sensitive. setUpOutputDir(baseName) mutates a global system property, and ResumeTest reads it back post-run to locate the state and .log files, with an assertTrue(logFiles.length == 1) check. Under Surefire parallel or forkCount execution, ExampleXmlParsingTest/ExampleJSONParsingTest (which set the prefix to test/) can clobber it mid-test, producing flaky "expected exactly 1 .log file" / "state file missing" failures. Your javadoc acknowledges the limitation. It is worth either documenting the required /sequential constraint in the pom or flagging it explicitly.

  4. XMLPathUtil.java:8 (and ResumeTest reuse) have stale javadoc. The class doc says "Shared test infrastructure for beast-fx integration tests", but this PR moves it into beast-base (test.beast.integration) and all its callers are now in beast-base. The doc misdirects a future maintainer to the wrong module.

  5. ResumeTest.java:62 A non-MCMC runnable yields a misleading failure. If any listed XML's resolves to a non-MCMC Runnable, the if (runable instanceof MCMC mcmc) block is skipped, nothing runs, and execution falls through to assertTrue(sf.exists(), "…state file missing"). This will report "state file missing" rather than "wasn't an MCMC". All five current XMLs are MCMC, so this is latent, not active.

  6. ResumeTest.java:57-70 vs 94-107 The initial-run and resume blocks are near-duplicates.
    They differ only in Logger.FILE_MODE(overwrite/resume) and the setStateFile resume flag; a parseAndRun(fileName, stateFile, mode, resume) helper would remove ~13 duplicated lines and the sync hazard.

@walterxie

Copy link
Copy Markdown
Member Author

I have resolved all the issues listed above; each commit generally corresponds to an issue in chronological order.

@walterxie walterxie merged commit 4554fa3 into master Jun 23, 2026
1 check passed
@walterxie walterxie deleted the xmltest branch June 25, 2026 00:02
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