[foreign-memaccess+abi] RFR: Improve test coverage in the jdk.internal.layout package

Jorn Vernee jvernee at openjdk.org
Mon Feb 13 16:49:45 UTC 2023


On Mon, 13 Feb 2023 14:13:37 GMT, Per Minborg <pminborg at openjdk.org> wrote:

> This PR adds some tests and minor code refactoring.

Marked as reviewed by jvernee (Committer).

test/jdk/java/foreign/TestLayouts.java line 80:

> 78:         assertFalse(ADDRESS.equals(differentTargetLayout));
> 79:         var equalButNotSame = ADDRESS.withTargetLayout(JAVA_INT).withTargetLayout(JAVA_CHAR);
> 80:         assertFalse(ADDRESS.equals(equalButNotSame));

I don't see what this extra code is testing that wasn't already tested by the lines above.

Was this intended as `assertTrue(differentTargetLayout .equals(equalButNotSame));` ?

test/jdk/java/foreign/TestLayouts.java line 131:

> 129:             assertFalse(layout.equals(other));
> 130:         }
> 131:     }

This tests mostly the same thing as `testNotEquals` so I suggest trying to unify the two.

Also, for the last 2 asserts, you could test structural equality for all test values by changing the parameter to a `Supplier<MemoryLayout>` which returns a new instance every time, and then simply call it twice to get 2 distinct, but structurally equal, instances.

test/jdk/java/foreign/TestLayouts.java line 146:

> 144:     public void testBasicIllegalAlignment(MemoryLayout layout) {
> 145:         layout.withBitAlignment(3);
> 146:     }

This looks the same as `testGroupIllegalAlignmentNotPowerOfTwo` above but with a different data provider. I suggest have a single test with a data provider that aggregates the other two.

-------------

PR: https://git.openjdk.org/panama-foreign/pull/790


More information about the panama-dev mailing list