[GitHub] maven-surefire pull request #171: [SUREFIRE-1445] Explicitly define Surefire...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] maven-surefire pull request #171: [SUREFIRE-1445] Explicitly define Surefire...

Tibor17-2
Github user acogoluegnes commented on a diff in the pull request:

    https://github.com/apache/maven-surefire/pull/171#discussion_r153745052
 
    --- Diff: maven-surefire-common/src/test/java/org/apache/maven/plugin/surefire/SurefirePropertiesTest.java ---
    @@ -70,9 +70,16 @@ public void testConstructWithOther()
             src.setProperty( "b", "2" );
             SurefireProperties orderedProperties = new SurefireProperties( src );
             // Cannot make assumptions about insertion order
    -        assertEquals( 2, orderedProperties.size() );
    -
    -
    +        int expectedCount = 0;
    +        // keys() uses the items property, more reliable to test than size(),
    +        // which is based on the Properties class
    +        // see https://issues.apache.org/jira/browse/SUREFIRE-1445
    +        Enumeration<Object> keys = orderedProperties.keys();
    +        while (keys.hasMoreElements()) {
    +            keys.nextElement();
    +            expectedCount++;
    +        }
    +        assertEquals( 2, expectedCount );
    --- End diff --
   
    I don't get it. I used `keys()` on purpose because it relies on `items`, which was not in-sync in the first place. `getStringKeySet()` relies `keySet()`, which uses the base class. Both would work, but `getStringKeySet()` doesn't catch the bug. Does that make sense?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]