Skip to content

Conversation

@lawrencewang49
Copy link

Purpose of this PR

This PR makes XML comparisons in tests deterministic by normalizing attribute order before asserting equality.
When running tests under NonDex, XML attributes may appear in different orders, causing false test failures even though the XML content is equivalent. This is likely due to the use of a hashMap in completeState() in EclipseApp.java when

args.asMap().forEach((key, values) -> {

is used to build the XML string, and the use of hashMaps in mirrorApp in P2Model.java

destination.attributes().put("location", ...)
destination.attributes().put("append", append);
...
iuNode.attributes().put("id", ...)
iuNode.attributes().put("version", ...)

from calling .attributes(), as HashMaps do not guarantee the order of key value pairs over time.
This class is confirmed when adding the following lines to mirrorApp :

System.out.println("destination attributes class = " + destination.attributes().getClass());
System.out.println("iuNode attributes class = " + iuNode.attributes().getClass());

This change ensures the XML tests fail only when there is a real XML mismatch, not due to nondeterministic attribute ordering.

What the PR changes

  • Replaces:
Assert.assertEquals(trim(expected), trim(actual));

with

Assert.assertEquals(normalizeXml(trim(expected)), normalizeXml(trim(actual)));
  • Adds normalizeXml to CleanedAssert.java, which:
    • detects XML tags
    • extracts all attributes
    • sorts attributes alphabetically
    • reconstructs the normalized XML string for comparison
      This ensures that
<repo kind="meta" location="url"/>

and

<repo location="url" kind="meta"/>

are treated as equal in tests

Results

  • Eliminates nondeterministic test failures under Nondex
  • Makes test suite deterministic and stable across environments
  • Improves confidence in XML-based tests by focusing on real semantic differences
  • Enhances reliability of CI pipeline when randomness is instroduced

Steps to reproduce original Failure

  1. Add the following to plugins in build.gradle: id 'edu.illinois.nondex' version '2.1.7'
  2. Add the following to the bottom of build.gradle: apply plugin: 'edu.illinois.nondex'
  3. Change
test {
	if(JavaVersion.current() != JavaVersion.VERSION_1_8) {
		jvmArgs '--add-modules=ALL-SYSTEM', '--add-opens=java.base/jdk.internal.loader=ALL-UNNAMED'
	}
	testLogging.exceptionFormat = 'full'
}

to

tasks.withType(Test).configureEach {
    if (JavaVersion.current() != JavaVersion.VERSION_1_8) {
        jvmArgs '--add-modules=ALL-SYSTEM', '--add-opens=java.base/jdk.internal.loader=ALL-UNNAMED'
    }
    testLogging {
        exceptionFormat = 'full'
        events "passed", "failed", "skipped", "standardOut", "standardError"
        showStandardStreams = true
    }
}
  1. Run ./gradlew nondexTest --tests "com.diffplug.gradle.p2.P2ModelTest.testMirrorAntFile",
    ./gradlew nondexTest --tests "com.diffplug.gradle.p2.P2ModelTest.testMirrorAntFileWithAppend",
    ./gradlew nondexTest --tests "com.diffplug.gradle.p2.P2ModelTest.testMirrorAntFileWithAppenDefault",
    and ./gradlew nondexTest --tests "com.diffplug.gradle.p2.P2ModelTest.testMirrorAntFileWithSlicingOptions" in the terminal
  • there should be output similar to what is shown below:
P2ModelTest > testMirrorAntFile FAILED
    org.junit.ComparisonFailure: expected:<...repo"/>
    <repository [kind="metadata" location="https://metadatarepo"/>
    <repository kind="artifact" location="https://artifactrepo"/>
    </source>
    <destination location="file:///home/lw41/progress_report_4/goomph/dest" append="false"/>
    <iu id="com.diffplug.iu"/>
    <iu id="com.diffplug.otheriu" version="1.0.0]"/>
    </p2.mirror>
    </p...> but was:<...repo"/>
    <repository [location="https://metadatarepo" kind="metadata"/>
    <repository location="https://artifactrepo" kind="artifact"/>
    </source>
    <destination append="false" location="file:///home/lw41/progress_report_4/goomph/dest"/>
    <iu id="com.diffplug.iu"/>
    <iu version="1.0.0" id="com.diffplug.otheriu]"/>
    </p2.mirror>
    </p...>
        at org.junit.Assert.assertEquals(Assert.java:117)
        at org.junit.Assert.assertEquals(Assert.java:146)
        at com.diffplug.gradle.CleanedAssert.xml(CleanedAssert.java:31)
        at com.diffplug.gradle.p2.P2ModelTest.testMirrorAntFile(P2ModelTest.java:79)

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.

1 participant