You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/05/20 00:19:04 UTC
[GitHub] [pinot] walterddr opened a new pull request, #8744: remove ControllerTestUtils static and use instance instead
walterddr opened a new pull request, #8744:
URL: https://github.com/apache/pinot/pull/8744
Background
===
Currently, we have `ControllerTestUtils` and `ControllerTest` which basically replicate functionality for test utilities. this
- merge avoids discrepancy between static test util and non-static controller test object
- give foundation for more extensible `ClusterTest` object for integration test. one step closer to shared integration test structure.
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8744: remove static ControllerTestUtils and use singleton instance ControllerTest instead
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8744:
URL: https://github.com/apache/pinot/pull/8744#discussion_r879856732
##########
pinot-compatibility-verifier/src/main/java/org/apache/pinot/compat/SegmentOp.java:
##########
@@ -294,7 +294,7 @@ private boolean deleteSegment() {
_segmentName = _tableName + "_" + _generationNumber;
}
- ControllerTest.sendDeleteRequest(
+ ControllerTest.getInstance().sendDeleteRequest(
Review Comment:
Revert the changes in this file
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotAccessControlUserRestletResourceTest.java:
##########
@@ -88,16 +89,16 @@ public void testUpdateUserConfig()
String username = "updateTC";
String userConfigString = _userConfigBuilder.setUsername(username).setComponentType(ComponentType.CONTROLLER)
.build().toJsonString();
- ControllerTestUtils.sendPostRequest(_createUserUrl, userConfigString);
+ ControllerTest.sendPostRequest(_createUserUrl, userConfigString);
// user creation should succeed
UserConfig userConfig = getUserConfig(username, "CONTROLLER");
Assert.assertEquals(userConfig.getRoleType().toString(), RoleType.USER.toString());
Assert.assertTrue(BcryptUtils.checkpw("123456", userConfig.getPassword()));
userConfig.setRole("ADMIN");
userConfig.setPassword("654321");
- JsonNode jsonResponse = JsonUtils.stringToJsonNode(ControllerTestUtils
- .sendPutRequest(ControllerTestUtils.getControllerRequestURLBuilder()
+ JsonNode jsonResponse = JsonUtils.stringToJsonNode(TEST_INSTANCE
Review Comment:
```suggestion
JsonNode jsonResponse = JsonUtils.stringToJsonNode(ControllerTest
```
##########
pinot-compatibility-verifier/src/main/java/org/apache/pinot/compat/Utils.java:
##########
@@ -57,6 +57,6 @@ public static JsonNode postSqlQuery(String query, String brokerBaseApiUrl)
payload.put("queryOptions", "groupByMode=sql;responseFormat=sql");
return JsonUtils.stringToJsonNode(
- ControllerTest.sendPostRequest(brokerBaseApiUrl + "/query/sql", payload.toString()));
+ ControllerTest.getInstance().sendPostRequest(brokerBaseApiUrl + "/query/sql", payload.toString()));
Review Comment:
Revert the changes in this file
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java:
##########
@@ -49,8 +50,8 @@ public void testPostJson() {
+ " \"metricFieldSpecs\" : [ {\n" + " \"name\" : \"score\",\n" + " \"dataType\" : \"FLOAT\"\n"
+ " } ]}";
try {
- final String response = ControllerTestUtils
- .sendPostRequest(ControllerTestUtils.getControllerRequestURLBuilder().forSchemaCreate(), schemaString);
+ final String response = TEST_INSTANCE
Review Comment:
```suggestion
final String response = ControllerTest
```
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotAccessControlUserRestletResourceTest.java:
##########
@@ -113,25 +114,25 @@ public void testDeleteUser()
// Case 1: Create a CONTORLLER user and delete it directly w/o using query param.
UserConfig controllerUserConfig = _userConfigBuilder.setUsername("user1")
.setComponentType(ComponentType.CONTROLLER).build();
- String creationResponse = ControllerTestUtils
+ String creationResponse = TEST_INSTANCE
Review Comment:
```suggestion
String creationResponse = ControllerTest
```
##########
pinot-compatibility-verifier/src/main/java/org/apache/pinot/compat/TableOp.java:
##########
@@ -109,7 +109,7 @@ private boolean createSchema() {
Map<String, String> headers = new HashMap<String, String>() {{
put("Content-type", "application/json");
}};
- ControllerTest.sendPostRequestRaw(
+ ControllerTest.getInstance().sendPostRequestRaw(
Review Comment:
Revert the changes in this file
##########
pinot-compatibility-verifier/src/main/java/org/apache/pinot/compat/StreamOp.java:
##########
@@ -222,8 +222,8 @@ private boolean produceData() {
String timeColumn = tableConfig.getValidationConfig().getTimeColumnName();
String schemaName = TableNameBuilder.extractRawTableName(tableName);
- String schemaString = ControllerTest.
- sendGetRequest(ControllerRequestURLBuilder.baseUrl(ClusterDescriptor.getInstance().getControllerUrl())
+ String schemaString = ControllerTest.getInstance()
Review Comment:
Revert the changes in 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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] walterddr merged pull request #8744: remove static ControllerTestUtils and use singleton instance ControllerTest instead
Posted by GitBox <gi...@apache.org>.
walterddr merged PR #8744:
URL: https://github.com/apache/pinot/pull/8744
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #8744: remove static ControllerTestUtils and use singleton instance ControllerTest instead
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8744:
URL: https://github.com/apache/pinot/pull/8744#issuecomment-1132369379
# [Codecov](https://codecov.io/gh/apache/pinot/pull/8744?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#8744](https://codecov.io/gh/apache/pinot/pull/8744?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (55ecf90) into [master](https://codecov.io/gh/apache/pinot/commit/c739f423eebfd9751c26f6aa410e5e8830ecde71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c739f42) will **decrease** coverage by `3.49%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #8744 +/- ##
============================================
- Coverage 69.66% 66.17% -3.50%
+ Complexity 4611 4427 -184
============================================
Files 1730 1284 -446
Lines 90314 64838 -25476
Branches 13421 10096 -3325
============================================
- Hits 62918 42907 -20011
+ Misses 23022 18871 -4151
+ Partials 4374 3060 -1314
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `66.17% <ø> (+<0.01%)` | :arrow_up: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8744?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...va/org/apache/pinot/core/routing/RoutingTable.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9yb3V0aW5nL1JvdXRpbmdUYWJsZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/common/config/NettyConfig.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vY29uZmlnL05ldHR5Q29uZmlnLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...g/apache/pinot/common/metrics/ControllerMeter.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Db250cm9sbGVyTWV0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...apache/pinot/common/helix/ExtraInstanceConfig.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vaGVsaXgvRXh0cmFJbnN0YW5jZUNvbmZpZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ache/pinot/server/access/AccessControlFactory.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL2FjY2Vzcy9BY2Nlc3NDb250cm9sRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/SegmentReloadMessage.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvU2VnbWVudFJlbG9hZE1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...he/pinot/common/messages/TableDeletionMessage.java](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWVzc2FnZXMvVGFibGVEZWxldGlvbk1lc3NhZ2UuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [676 more](https://codecov.io/gh/apache/pinot/pull/8744/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8744?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8744?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [c739f42...55ecf90](https://codecov.io/gh/apache/pinot/pull/8744?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org