This is an automated email from the ASF dual-hosted git repository.
xiaokang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-graphar.git
The following commit(s) were added to refs/heads/main by this push:
new 914d065a feat(java,info): use assert throw and adjust version check
logic (#796)
914d065a is described below
commit 914d065a5da9faf3e03d8c859492731ffa4f1b10
Author: Xiaokang Yang <[email protected]>
AuthorDate: Thu Oct 30 17:34:54 2025 +0800
feat(java,info): use assert throw and adjust version check logic (#796)
* use assertThrow
* Align c++ version check and fix emptyPropertyGroupsEdgeBuilderTest
* re add removed test method
---
.../java/org/apache/graphar/info/VersionInfo.java | 10 ++--
.../java/org/apache/graphar/info/EdgeInfoTest.java | 68 ++++++++++++----------
.../org/apache/graphar/info/PropertyGroupTest.java | 35 +++++------
.../org/apache/graphar/info/VersionInfoTest.java | 45 ++++++--------
.../org/apache/graphar/info/VertexInfoTest.java | 12 ++--
5 files changed, 82 insertions(+), 88 deletions(-)
diff --git
a/maven-projects/info/src/main/java/org/apache/graphar/info/VersionInfo.java
b/maven-projects/info/src/main/java/org/apache/graphar/info/VersionInfo.java
index 3d7be11b..6e717b8a 100644
--- a/maven-projects/info/src/main/java/org/apache/graphar/info/VersionInfo.java
+++ b/maven-projects/info/src/main/java/org/apache/graphar/info/VersionInfo.java
@@ -67,13 +67,13 @@ public class VersionInfo {
/** Check if type is supported by version. */
public boolean checkType(final String typeStr) {
- if (version2types == null || !version2types.containsKey(version)) {
- return false;
- }
+ // Check built-in types first if version is supported
List<String> types = version2types.get(version);
- if (types.contains(typeStr)) {
+ if (types != null && types.contains(typeStr)) {
return true;
}
- return userDefinedTypes != null && userDefinedTypes.contains(typeStr);
+
+ // Check user-defined types
+ return userDefinedTypes.contains(typeStr);
}
}
diff --git
a/maven-projects/info/src/test/java/org/apache/graphar/info/EdgeInfoTest.java
b/maven-projects/info/src/test/java/org/apache/graphar/info/EdgeInfoTest.java
index 7ba683bf..da64cf01 100644
---
a/maven-projects/info/src/test/java/org/apache/graphar/info/EdgeInfoTest.java
+++
b/maven-projects/info/src/test/java/org/apache/graphar/info/EdgeInfoTest.java
@@ -28,17 +28,17 @@ import org.junit.Assert;
import org.junit.Test;
public class EdgeInfoTest {
-
- private final EdgeInfo.EdgeInfoBuilder e =
- EdgeInfo.builder()
- .srcType("person")
- .edgeType("knows")
- .chunkSize(1024)
- .srcChunkSize(100)
- .dstChunkSize(100)
- .directed(false)
- .baseUri(URI.create("edge/person_knows_person/"))
- .version("gar/v1");
+ private EdgeInfo.EdgeInfoBuilder createBaseEdgeInfoBuilder() {
+ return EdgeInfo.builder()
+ .srcType("person")
+ .edgeType("knows")
+ .chunkSize(1024)
+ .srcChunkSize(100)
+ .dstChunkSize(100)
+ .directed(false)
+ .baseUri(URI.create("edge/person_knows_person/"))
+ .version("gar/v1");
+ }
@Test
public void testBuildWithPrefix() {
@@ -108,40 +108,43 @@ public class EdgeInfoTest {
@Test
public void erroneousTripletEdgeBuilderTest() {
- try {
- e.adjacentLists(List.of(TestUtil.orderedBySource,
TestUtil.orderedByDest))
- .addPropertyGroups(List.of(TestUtil.pg3))
- .build();
- } catch (IllegalArgumentException e) {
- Assert.assertEquals("Edge triplet is null", e.getMessage());
- }
+ EdgeInfo.EdgeInfoBuilder edgeInfoBuilder =
+ createBaseEdgeInfoBuilder()
+ .adjacentLists(List.of(TestUtil.orderedBySource,
TestUtil.orderedByDest))
+ .addPropertyGroups(List.of(TestUtil.pg3));
+ IllegalArgumentException illegalArgumentException =
+ Assert.assertThrows(IllegalArgumentException.class,
edgeInfoBuilder::build);
+ Assert.assertEquals("Edge triplet is null",
illegalArgumentException.getMessage());
}
@Test
public void emptyAdjacentListEdgeBuilderTest() {
- try {
-
e.dstType("person").addPropertyGroups(List.of(TestUtil.pg3)).build();
- } catch (IllegalArgumentException e) {
- Assert.assertEquals("AdjacentLists is empty", e.getMessage());
- }
+ EdgeInfo.EdgeInfoBuilder edgeInfoBuilder =
+ createBaseEdgeInfoBuilder()
+ .dstType("person")
+ .addPropertyGroups(List.of(TestUtil.pg3));
+ IllegalArgumentException illegalArgumentException =
+ Assert.assertThrows(IllegalArgumentException.class,
edgeInfoBuilder::build);
+ Assert.assertEquals("AdjacentLists is empty",
illegalArgumentException.getMessage());
}
@Test
public void emptyPropertyGroupsEdgeBuilderTest() {
- try {
- e.adjacentLists(List.of(TestUtil.orderedBySource,
TestUtil.orderedByDest))
- .dstType("person")
- .build();
- } catch (IllegalArgumentException e) {
- Assert.assertEquals("PropertyGroups is empty", e.getMessage());
- }
+ EdgeInfo.EdgeInfoBuilder edgeInfoBuilder =
+ createBaseEdgeInfoBuilder()
+ .dstType("person")
+ .adjacentLists(List.of(TestUtil.orderedBySource,
TestUtil.orderedByDest));
+ IllegalArgumentException illegalArgumentException =
+ Assert.assertThrows(IllegalArgumentException.class,
edgeInfoBuilder::build);
+ Assert.assertEquals("PropertyGroups is empty",
illegalArgumentException.getMessage());
}
@Test
public void addMethodsTest() {
EdgeInfo edgeInfo =
- e.addPropertyGroup(TestUtil.pg3)
+ createBaseEdgeInfoBuilder()
+ .addPropertyGroup(TestUtil.pg3)
.addAdjacentList(TestUtil.orderedBySource)
.addAdjacentList(TestUtil.orderedByDest)
.dstType("person")
@@ -154,7 +157,8 @@ public class EdgeInfoTest {
@Test
public void appendMethodsTest() {
EdgeInfo edgeInfo =
- e.propertyGroups(new PropertyGroups(List.of(TestUtil.pg3)))
+ createBaseEdgeInfoBuilder()
+ .propertyGroups(new
PropertyGroups(List.of(TestUtil.pg3)))
.adjacentLists(List.of(TestUtil.orderedBySource))
.addAdjacentList(TestUtil.orderedByDest)
.addPropertyGroups(List.of(TestUtil.pg2))
diff --git
a/maven-projects/info/src/test/java/org/apache/graphar/info/PropertyGroupTest.java
b/maven-projects/info/src/test/java/org/apache/graphar/info/PropertyGroupTest.java
index eaa0cc33..48346577 100644
---
a/maven-projects/info/src/test/java/org/apache/graphar/info/PropertyGroupTest.java
+++
b/maven-projects/info/src/test/java/org/apache/graphar/info/PropertyGroupTest.java
@@ -227,23 +227,24 @@ public class PropertyGroupTest {
Assert.assertEquals(1, pg.getPropertyList().size());
// Try to modify returned lists - should throw exception or be
unmodifiable
- try {
- pg.getPropertyList()
- .add(TestDataFactory.createProperty("another",
DataType.BOOL, false, false));
- Assert.fail("Should not be able to modify property list");
- } catch (UnsupportedOperationException expected) {
- // This is expected behavior
- }
-
- try {
- pg.getPropertyMap()
- .put(
- "test2",
- TestDataFactory.createProperty("test2",
DataType.FLOAT, false, false));
- Assert.fail("Should not be able to modify property map");
- } catch (UnsupportedOperationException expected) {
- // This is expected behavior
- }
+ UnsupportedOperationException unsupportedOperationException1 =
+ Assert.assertThrows(
+ UnsupportedOperationException.class,
+ () ->
+ pg.getPropertyList()
+ .add(
+ TestDataFactory.createProperty(
+ "another",
DataType.BOOL, false, false)));
+
+ UnsupportedOperationException unsupportedOperationException2 =
+ Assert.assertThrows(
+ UnsupportedOperationException.class,
+ () ->
+ pg.getPropertyMap()
+ .put(
+ "test2",
+ TestDataFactory.createProperty(
+ "test2",
DataType.FLOAT, false, false)));
}
@Test
diff --git
a/maven-projects/info/src/test/java/org/apache/graphar/info/VersionInfoTest.java
b/maven-projects/info/src/test/java/org/apache/graphar/info/VersionInfoTest.java
index fe3d22df..1766fa43 100644
---
a/maven-projects/info/src/test/java/org/apache/graphar/info/VersionInfoTest.java
+++
b/maven-projects/info/src/test/java/org/apache/graphar/info/VersionInfoTest.java
@@ -114,12 +114,9 @@ public class VersionInfoTest {
VersionInfo versionInfo = new VersionInfo(1, null);
// The current implementation doesn't handle null gracefully, so this
will throw NPE
- try {
- versionInfo.checkType(null);
- Assert.fail("Expected NullPointerException");
- } catch (NullPointerException expected) {
- // This is the current behavior - null parameter causes NPE
- }
+ NullPointerException nullPointerException =
+ Assert.assertThrows(NullPointerException.class, () ->
versionInfo.checkType(null));
+ // This is the current behavior - null parameter causes NPE
}
@Test
@@ -127,13 +124,10 @@ public class VersionInfoTest {
List<String> userTypes = Arrays.asList("customType");
VersionInfo versionInfo = new VersionInfo(999, userTypes);
- // Unsupported version should not support built-in types
Assert.assertFalse(versionInfo.checkType("int32"));
Assert.assertFalse(versionInfo.checkType("string"));
- // Current implementation: unsupported version returns false for all
types,
- // including user-defined types, due to early return in checkType()
- Assert.assertFalse(versionInfo.checkType("customType"));
+ Assert.assertTrue(versionInfo.checkType("customType"));
}
@Test
@@ -168,26 +162,25 @@ public class VersionInfoTest {
@Test
public void testVersionInfoWithZeroVersion() {
- try {
- new VersionInfo(0, null);
- Assert.fail("Expected IllegalArgumentException for zero version");
- } catch (IllegalArgumentException e) {
- Assert.assertTrue(
- "Error message should mention version",
- e.getMessage().contains("Version must be a supported
positive integer"));
- }
+ IllegalArgumentException illegalArgumentException =
+ Assert.assertThrows(IllegalArgumentException.class, () -> new
VersionInfo(0, null));
+ Assert.assertTrue(
+ "Error message should mention version",
+ illegalArgumentException
+ .getMessage()
+ .contains("Version must be a supported positive
integer"));
}
@Test
public void testVersionInfoWithNegativeVersion() {
- try {
- new VersionInfo(-1, null);
- Assert.fail("Expected IllegalArgumentException for negative
version");
- } catch (IllegalArgumentException e) {
- Assert.assertTrue(
- "Error message should mention version",
- e.getMessage().contains("Version must be a supported
positive integer"));
- }
+ IllegalArgumentException illegalArgumentException =
+ Assert.assertThrows(
+ IllegalArgumentException.class, () -> new
VersionInfo(-1, null));
+ Assert.assertTrue(
+ "Error message should mention version",
+ illegalArgumentException
+ .getMessage()
+ .contains("Version must be a supported positive
integer"));
}
@Test
diff --git
a/maven-projects/info/src/test/java/org/apache/graphar/info/VertexInfoTest.java
b/maven-projects/info/src/test/java/org/apache/graphar/info/VertexInfoTest.java
index 4623c738..b4624415 100644
---
a/maven-projects/info/src/test/java/org/apache/graphar/info/VertexInfoTest.java
+++
b/maven-projects/info/src/test/java/org/apache/graphar/info/VertexInfoTest.java
@@ -92,14 +92,10 @@ public class VertexInfoTest {
@Test
public void testBuildWithPrefix() {
- try {
- VertexInfo vertexInfo =
- new VertexInfo(
- "person", 100, Arrays.asList(TestUtil.pg1),
"vertex/person/", "gar/v1");
- Assert.assertEquals(URI.create("vertex/person/"),
vertexInfo.getBaseUri());
- } catch (Exception e) {
- Assert.fail("Should not throw exception: " + e.getMessage());
- }
+ VertexInfo vertexInfo =
+ new VertexInfo(
+ "person", 100, Arrays.asList(TestUtil.pg1),
"vertex/person/", "gar/v1");
+ Assert.assertEquals(URI.create("vertex/person/"),
vertexInfo.getBaseUri());
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
