[
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)