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.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
