[ 
https://issues.apache.org/jira/browse/PHOENIX-7779?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18072343#comment-18072343
 ] 

ASF GitHub Bot commented on PHOENIX-7779:
-----------------------------------------

richardantal commented on code in PR #189:
URL: 
https://github.com/apache/phoenix-queryserver/pull/189#discussion_r3058668095


##########
phoenix-queryserver-it/src/it/java/org/apache/phoenix/tool/ParameterizedPhoenixCanaryToolIT.java:
##########
@@ -76,36 +74,28 @@ public class ParameterizedPhoenixCanaryToolIT extends 
BaseTest {
        private ByteArrayOutputStream out = new ByteArrayOutputStream();
     private String tmpDir = System.getProperty("java.io.tmpdir");
 
-       public ParameterizedPhoenixCanaryToolIT(boolean isPositiveTestType,
-                       boolean isNamespaceEnabled, String resultSinkOption) {
-               this.isPositiveTestType = isPositiveTestType;
-               this.isNamespaceEnabled = isNamespaceEnabled;
-               this.resultSinkOption = resultSinkOption;
-       }
-
-       @Parameterized.Parameters(name = 
"ParameterizedPhoenixCanaryToolIT_isPositiveTestType={0}," +
-                       "isNamespaceEnabled={1},resultSinkOption={2}")
-       public static Collection parametersList() {
-               return Arrays.asList(new Object[][] {
-                       {true, true, stdOutSink},
-                       {true, true, fileOutSink},
-                       {false, true, stdOutSink},
-                       {false, true, fileOutSink},
-                       {true, false, stdOutSink},
-                       {true, false, fileOutSink},
-                       {false, false, stdOutSink},
-                       {false, false, fileOutSink}
-               });
-       }
+    public static Stream<Arguments> parametersList() {
+        return Stream.of(
+                Arguments.arguments(true, true, stdOutSink),
+                Arguments.arguments(true, true, fileOutSink),
+                Arguments.arguments(false, true, stdOutSink),
+                Arguments.arguments(false, true, fileOutSink),
+                Arguments.arguments(true, false, stdOutSink),
+                Arguments.arguments(true, false, fileOutSink),
+                Arguments.arguments(false, false, stdOutSink),
+                Arguments.arguments(false, false, fileOutSink)
+        );
+    }
 
-       @Before
-       public void setup() throws Exception {
+       public void setup(
+            Boolean isPositiveTestType, Boolean isNamespaceEnabled, String 
resultSinkOption) throws Exception {
                String createSchema;
                String createTable;
+               cmd.clear();
 
-               if(needsNewCluster()) {
-                       setClientSideNamespaceProperties();
-                       setServerSideNamespaceProperties();
+               if(needsNewCluster(isNamespaceEnabled)) {

Review Comment:
   Can we use this.isNamespaceEnabled instead of passing parameters?



##########
phoenix-queryserver-it/src/it/java/org/apache/phoenix/tool/ParameterizedPhoenixCanaryToolIT.java:
##########
@@ -115,14 +105,14 @@ public void setup() throws Exception {
                        setUpTestDriver(new 
ReadOnlyProps(serverProps.entrySet().iterator()),
                                        new 
ReadOnlyProps(clientProps.entrySet().iterator()));
                        LOGGER.info("New cluster is spinned up with test 
parameters " +
-                                       "isPositiveTestType" + 
this.isPositiveTestType +
-                                       "isNamespaceEnabled" + 
this.isNamespaceEnabled +
-                                       "resultSinkOption" + 
this.resultSinkOption);
+                                       "isPositiveTestType" + 
isPositiveTestType +
+                                       "isNamespaceEnabled" + 
isNamespaceEnabled +
+                                       "resultSinkOption" + resultSinkOption);

Review Comment:
   NIT
   This is not related to your change, but it would be nice to add extra spaces 
to make this LOG line readable



##########
phoenix-queryserver-it/src/it/java/org/apache/phoenix/tool/ParameterizedPhoenixCanaryToolIT.java:
##########
@@ -76,36 +74,28 @@ public class ParameterizedPhoenixCanaryToolIT extends 
BaseTest {
        private ByteArrayOutputStream out = new ByteArrayOutputStream();
     private String tmpDir = System.getProperty("java.io.tmpdir");
 
-       public ParameterizedPhoenixCanaryToolIT(boolean isPositiveTestType,
-                       boolean isNamespaceEnabled, String resultSinkOption) {
-               this.isPositiveTestType = isPositiveTestType;
-               this.isNamespaceEnabled = isNamespaceEnabled;
-               this.resultSinkOption = resultSinkOption;
-       }
-
-       @Parameterized.Parameters(name = 
"ParameterizedPhoenixCanaryToolIT_isPositiveTestType={0}," +
-                       "isNamespaceEnabled={1},resultSinkOption={2}")
-       public static Collection parametersList() {
-               return Arrays.asList(new Object[][] {
-                       {true, true, stdOutSink},
-                       {true, true, fileOutSink},
-                       {false, true, stdOutSink},
-                       {false, true, fileOutSink},
-                       {true, false, stdOutSink},
-                       {true, false, fileOutSink},
-                       {false, false, stdOutSink},
-                       {false, false, fileOutSink}
-               });
-       }
+    public static Stream<Arguments> parametersList() {
+        return Stream.of(
+                Arguments.arguments(true, true, stdOutSink),
+                Arguments.arguments(true, true, fileOutSink),
+                Arguments.arguments(false, true, stdOutSink),
+                Arguments.arguments(false, true, fileOutSink),
+                Arguments.arguments(true, false, stdOutSink),
+                Arguments.arguments(true, false, fileOutSink),
+                Arguments.arguments(false, false, stdOutSink),
+                Arguments.arguments(false, false, fileOutSink)
+        );
+    }
 
-       @Before
-       public void setup() throws Exception {
+       public void setup(
+            Boolean isPositiveTestType, Boolean isNamespaceEnabled, String 
resultSinkOption) throws Exception {

Review Comment:
   Can we use this.isNamespaceEnabled, this.resultSinkOption and 
this.isPositiveTestType instead of passing parameters?
   This applies to most of the places where we use isNamespaceEnabled, 
resultSinkOption and isPositiveTestType this file.





> Move phoenix-queryserver to jUnit 5
> -----------------------------------
>
>                 Key: PHOENIX-7779
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-7779
>             Project: Phoenix
>          Issue Type: Improvement
>          Components: queryserver
>            Reporter: Tamas Penzes
>            Assignee: Alexandra Dunai
>            Priority: Major
>
> HBase is moving towards jUnit5, it's time to do the same for Phoenix too.
> Let's start with Queryserver first.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to