Skip to content

Convert dom.mem.Test to JUnit - #104

Open
elharo wants to merge 3 commits into
mainfrom
convert-mem-to-junit
Open

Convert dom.mem.Test to JUnit#104
elharo wants to merge 3 commits into
mainfrom
convert-mem-to-junit

Conversation

@elharo

@elharo elharo commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Convert the dom.mem.Test test harness from a main()-driven to JUnit 3.

  • Extends TestCase
  • Replaces broken Assertion.verify/Assertion.equals with proper JUnit assertions throughout all DOM Level 2/3 tests (1395 lines)
  • Registered in batchtest and old java task removed in build.xml

@elharo
elharo force-pushed the convert-mem-to-junit branch from d52fa76 to 95b8ea5 Compare July 7, 2026 22:04
@elharo elharo changed the title [XERCESJ-1796] Convert dom.mem.Test to JUnit 3 Convert dom.mem.Test to JUnit 3 Jul 8, 2026
@elharo
elharo requested a review from garydgregory July 8, 2026 12:51
@elharo elharo changed the title Convert dom.mem.Test to JUnit 3 Convert dom.mem.Test to JUnit Jul 8, 2026
@elharo
elharo force-pushed the convert-mem-to-junit branch from 023d3b2 to a546f86 Compare July 16, 2026 14:41
@elharo
elharo requested a review from mrglavas July 16, 2026 18:50
Comment thread tests/dom/mem/Test.java Outdated
Comment on lines +369 to +370
assertNull(nnm);
assertNull(nnm);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we checking this twice?

Comment thread tests/dom/mem/Test.java Outdated
Comment on lines +439 to +440
assertNull(nl);
assertNull(nl);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here about redundant checks.

Comment thread tests/dom/mem/Test.java Outdated
Assertion.verify(dst != null &&
dst.getNodeType() == fNode.getNodeType());
assertEquals(UserDataHandler.NODE_CLONED, operation);
assertTrue(key == fKey && data == fData && src == fNode);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be split into three assertEquals or assertSame checks

Comment thread tests/dom/mem/Test.java Outdated
dst.getNodeType() == fNode.getNodeType());
assertEquals(UserDataHandler.NODE_CLONED, operation);
assertTrue(key == fKey && data == fData && src == fNode);
assertTrue(dst != null && dst.getNodeType() == fNode.getNodeType());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be one assertEquals check. null check does not need to be explicit

@elharo

elharo commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Addressed review comments:

  • Duplicate assertNull checks removed (lines 370, 440)
  • Compound assertTrue on line 1159 split into three assertSame calls
  • Compound assertTrue on line 1160 replaced with assertEquals (null check implicit via NPE)

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