Improve HL7v2 loose parser test coverage by NicoPiel · Pull Request #399 · OpenIntegrationEngine/engine

NicoPiel · 2026-07-30T18:08:33Z

Adds comprehensive unit test coverage and supporting tweaks for the HL7v2 loose serializer/parser:

Introduces a TestUtil.normalizeLineEndings helper to make tests resilient across platforms.
Expands ER7Serializer tests to cover:
- transformWithoutSerializing behavior for various delimiter and convertLineBreaks configurations (including the delimiter-replace branch).
- handling of external DTDs and SAX errors.
- encoding character handling, trailing separators, empty segments, and header-only messages.
Renames and refreshes the HL7 serializer test suite, normalizing line endings in assertions and re-enabling an orphaned loose-serializer test run.
Adds a round-trip invariant test asserting ER7 -> XML -> ER7 preserves message content (with normalized line endings).
Updates expected HL7 XML fixtures to reflect default subcomponent-handling behavior.

Signed-off-by: Nico Piel <nico.piel@hotmail.de>

HL7SerializerTests never matched Gradle's '**/*Test.class' filter, so 15 loose-parser round-trip tests have never run in CI. Rename it, and compare ER7-to-XML output with line endings normalized: the fixtures are CRLF while prettyPrintXml emits the platform separator, so these five assertions could only ever have passed on Windows. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

These fixtures were generated when handleSubcomponents defaulted to false. It now defaults to true, so the unescaped '&' in OBR.4.3 splits into subcomponents. Splitting is the spec-correct reading; the fixtures were stale. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

Covers custom MSH-2 separators, a header-only message with no trailing field separator, the MIRTH-1544 fixup firing on a non-header first segment, and the too-short-message guard. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

Consecutive and trailing repetition separators, trailing empty fields, and empty segments. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

Delimiter conversion, mixed line-break normalization, the pass-through case when delimiters already match, and the null "nothing to do" result. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

The four existing transformWithoutSerializing tests all set convertLineBreaks true (or leave the default serialization delimiter equal to the deserialization one), so every one of them short-circuits before reaching the StringUtils.replace branch. Add an overload of serializerWith that also controls the serialization delimiter, and a test with convertLineBreaks off and differing delimiters to actually exercise that branch. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

fromXML(toXML(er7)) must reproduce the input for every ER7 fixture. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

Remove a dead 3-argument serializerWith overload that duplicated the 2-argument form's behavior (the extra parameter always received the already-default segment delimiter), rename shadowed local `serializer` variables to `sameEnds` in the transformWithoutSerializing tests, and hoist the configuration-invariant ER7Serializer construction out of the fixture loop in testRoundTripAllEr7Fixtures. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

github-actions · 2026-07-30T18:09:31Z

Test Results

684 tests +29 684 ✅ +29 2m 2s ⏱️ +48s
110 suites + 1 0 💤 ± 0
110 files + 1 0 ❌ ± 0

Results for commit e353553. ± Comparison against base commit 776690e.

♻️ This comment has been updated with latest results.

Signed-off-by: Nico Piel <nico.piel@hotmail.de>

jonbartels · 2026-07-31T12:38:44Z

+     * in CRLF fixture files compare equal to output whose line separator is platform-dependent.
+     */
+    public static String normalizeLineEndings(String input) {
+        return input.replaceAll("\r\n|\r|\n", "\n").trim();


Non-blocking comment - Is there an Apache Commons method for this?

No. Checked lang3, commons-io and commons-text.

tonygermano

Thanks for re-enabling and improving upon some of these old tests.

Would it be possible to rebase and clean up the commit history a bit? My (and AIs) suggestion would be

The goal of the rebase is just to create a clean, atomic history of logical steps, rather than a literal recording of your local trial-and-error.

An ideal commit history for this PR would look something like this (4 logical commits):

Fix orphaned HL7v2 test suite and stale fixtures (Squash of your original commits 02 + 03)

test: cover HL7v2 loose parser encoding and degenerate separators (Squash of your original commits 04 + 05)

test: cover ER7Serializer.transformWithoutSerializing (Squash of your original commits 06 + 07 + 09)

test: assert HL7v2 loose parser ER7 round-trip invariant (Your original commit 08)

This keeps the repository history incredibly readable, preserves your excellent commit messages, and perfectly isolates each area of testing you improved.

Commits 1 and 10 would be dropped.

tonygermano · 2026-07-31T07:37:25Z

+				+ "<ZZZ.1><ZZZ.1.1><ZZZ.1.1.1></ZZZ.1.1.1><ZZZ.1.1.2></ZZZ.1.1.2></ZZZ.1.1></ZZZ.1>"
+				+ "<ZZZ.2><ZZZ.2.1><ZZZ.2.1.1>a</ZZZ.2.1.1><ZZZ.2.1.2>b</ZZZ.2.1.2></ZZZ.2.1></ZZZ.2>"
+				+ "</ZZZ></HL7Message>",
+				serializer.toXML("ZZZ|^~&|a&b"));


suggestion (non-blocking): update comment to describe expected behavior

I thought there was a bug in your test until I found the "MIRTH-1544" code that tests for the string your comment already calls out (^~&|) and then treats it as if the escape character is \ and missing.

tonygermano · 2026-07-31T15:13:50Z

+                <OBR.4.3.1>Jane MRI W/</OBR.4.3.1>
+                <OBR.4.3.2>W/O CONTRAST</OBR.4.3.2>


comment (non-blocking): ouch

Apparently the default a long time ago used to be that "parse sub-components" was off. Since the opposite is true now, I understand why this was necessary to fix the test.

But it pains me to look at because, aside from this data strangely including a name mixed with procedure data in OBR-4, I deal with the pain on a regular basis of radiology procedures containing & in the description and not being escaped in the hl7 messages.

NicoPiel added 9 commits July 30, 2026 18:45

Add SonarQube

9a93c48

Signed-off-by: Nico Piel <nico.piel@hotmail.de>

test: cover HL7v2 loose parser encoding-character handling

e9e7443

Covers custom MSH-2 separators, a header-only message with no trailing field separator, the MIRTH-1544 fixup firing on a non-header first segment, and the too-short-message guard. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

test: cover HL7v2 loose parser degenerate separators

efcc525

Consecutive and trailing repetition separators, trailing empty fields, and empty segments. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

test: cover ER7Serializer.transformWithoutSerializing

13a0641

Delimiter conversion, mixed line-break normalization, the pass-through case when delimiters already match, and the null "nothing to do" result. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

test: assert HL7v2 loose parser ER7 round-trip invariant

70a3bbe

fromXML(toXML(er7)) must reproduce the input for every ER7 fixture. Signed-off-by: Nico Piel <nico.piel@hotmail.de>

NicoPiel requested review from a team, gibson9583, jonbartels, kayyagari, kpalang, mgaffigan, ssrowe and tonygermano July 30, 2026 18:08

Remove SonarQube plugin from build.gradle

e353553

Signed-off-by: Nico Piel <nico.piel@hotmail.de>

mgaffigan approved these changes Jul 30, 2026

View reviewed changes

jonbartels reviewed Jul 31, 2026

View reviewed changes

kryskool approved these changes Jul 31, 2026

View reviewed changes

tonygermano requested changes Jul 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve HL7v2 loose parser test coverage - #399

Improve HL7v2 loose parser test coverage#399
NicoPiel wants to merge 10 commits into
OpenIntegrationEngine:mainfrom
NicoPiel:test/hl7v2-loose-coverage

NicoPiel commented Jul 30, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 30, 2026 •

edited

Loading

Uh oh!

jonbartels Jul 31, 2026

Uh oh!

NicoPiel Jul 31, 2026

Uh oh!

tonygermano left a comment

Uh oh!

tonygermano Jul 31, 2026

Uh oh!

tonygermano Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		<OBR.4.3.1>Jane MRI W/</OBR.4.3.1>
		<OBR.4.3.2>W/O CONTRAST</OBR.4.3.2>

Uh oh!

Conversation

NicoPiel commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

jonbartels Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

NicoPiel Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

tonygermano left a comment

Choose a reason for hiding this comment

Uh oh!

tonygermano Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

tonygermano Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

NicoPiel commented Jul 30, 2026 •

edited

Loading

github-actions Bot commented Jul 30, 2026 •

edited

Loading