justinmclean commented on code in PR #6098:
URL: https://github.com/apache/gravitino/pull/6098#discussion_r1903166510


##########
clients/cli/src/test/java/org/apache/gravitino/cli/TestParseType.java:
##########
@@ -19,49 +19,62 @@
 
 package org.apache.gravitino.cli;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.MatcherAssert.assertThat;
 
+import org.apache.gravitino.rel.types.Type;
+import org.apache.gravitino.rel.types.Types;
 import org.junit.jupiter.api.Test;
 
 public class TestParseType {
 
   @Test
-  public void testParseVarcharWithLength() {
-    ParsedType parsed = ParseType.parse("varchar(10)");
-    assertNotNull(parsed);
-    assertEquals("varchar", parsed.getTypeName());
-    assertEquals(10, parsed.getLength());
-    assertNull(parsed.getScale());
-    assertNull(parsed.getPrecision());
+  public void testParseTypeVarcharWithLength() {
+    Type type = ParseType.toType("varchar(10)");
+    assertThat(type, instanceOf(Types.VarCharType.class));
+    assertEquals(10, ((Types.VarCharType) type).length());
   }
 
   @Test
-  public void testParseDecimalWithPrecisionAndScale() {
-    ParsedType parsed = ParseType.parse("decimal(10,5)");
-    assertNotNull(parsed);
-    assertEquals("decimal", parsed.getTypeName());
-    assertEquals(10, parsed.getPrecision());
-    assertEquals(5, parsed.getScale());
-    assertNull(parsed.getLength());
+  public void testParseTypeDecimalWithPrecisionAndScale() {
+    Type type = ParseType.toType("decimal(10,5)");
+    assertThat(type, instanceOf(Types.DecimalType.class));
+    assertEquals(10, ((Types.DecimalType) type).precision());
+    assertEquals(5, ((Types.DecimalType) type).scale());
   }
 
   @Test
-  public void testParseIntegerWithoutParameters() {
-    ParsedType parsed = ParseType.parse("int()");
-    assertNull(parsed); // Expect null because the format is unsupported
+  public void testParseTypeList() {
+    Type type = ParseType.toType("list<integer>");
+    assertThat(type, instanceOf(Types.ListType.class));
+    Type elementType = ((Types.ListType) type).elementType();
+    assertThat(elementType, instanceOf(Types.IntegerType.class));
   }
 
   @Test
-  public void testParseOrdinaryInput() {
-    assertNull(ParseType.parse("string"));
-    assertNull(ParseType.parse("int"));
+  public void testParseTypeMap() {
+    Type type = ParseType.toType("map<string,integer>");
+    assertThat(type, instanceOf(Types.MapType.class));
+    Type keyType = ((Types.MapType) type).keyType();
+    Type valueType = ((Types.MapType) type).valueType();
+    assertThat(keyType, instanceOf(Types.StringType.class));
+    assertThat(valueType, instanceOf(Types.IntegerType.class));
   }
 
   @Test
-  public void testParseMalformedInput() {
-    assertNull(ParseType.parse("varchar(-10)"));
-    assertNull(ParseType.parse("decimal(10,abc)"));
+  public void testParseTypeIntegerWithoutParameters() {
+    assertThrows(IllegalArgumentException.class, () -> 
ParseType.toType("int()"));
+  }
+
+  @Test
+  public void testParseTypeOrdinaryInput() {
+    assertNull(ParseType.parseBasicType("string"));
+    assertNull(ParseType.parseBasicType("int"));
+  }
+
+  @Test
+  public void testParseTypeMalformedInput() {
+    assertThrows(IllegalArgumentException.class, () -> 
ParseType.toType("varchar(-10)"));
+    assertThrows(IllegalArgumentException.class, () -> 
ParseType.toType("decimal(10,abc)"));

Review Comment:
   It would be good to add some more tests here testing things like list(10), 
list(unknown), list(integer,integer), list(integer etc



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to