You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2024/03/19 05:06:25 UTC

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/21162


Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................

IMPALA-12898: Tidy up test dimensions of test_scanner.py

This patch tidies up the test dimensions of test_scanner.py.
'exec_option' initialization is moved to add_test_dimensions() method as
much as possible. It ensures correct permutation and execution of test
cases.

After this patch, the total collected tests of test_scanner.py is 993
for core/pairwise exploration and 7514 for exhaustive exploration.
Before, they were 794 and 11864 accordingly. The increase in test count
after refactoring with core exploration is because exec option
dimensions are now permuted correctly along with other default exec
option dimensions. The reduction in exhaustive exploration is due to a
reduction in the overall dimension to permute and a reduction in test
skipping (the test was run, but only called pytest.skip()).

Testing:
- Pass query_test/test_scanners.py in exhaustive exploration.

Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
---
M tests/query_test/test_scanners.py
1 file changed, 99 insertions(+), 89 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/21162/1
-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 4: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2024 18:18:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 3: Code-Review+2

Thanks for applying the changes! LGTM!


-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2024 10:35:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 3:

(17 comments)

http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@74
PS1, Line 74: 
> I think this is OK in core, because exhaustive tests still permute with lar
Reverted.


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@81
PS1, Line 81: lassmethod
> Same as above, earlier it was [0, 1, 16] in core.
Reverted.


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@100
PS1, Line 100: se
> nit: 4 spaces are needed
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@157
PS1, Line 157:   
> nit: 4 spaces are needed
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@201
PS1, Line 201: s 
> nit: indentation is off
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@203
PS1, Line 203: f 
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@259
PS1, Line 259: f 
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@353
PS1, Line 353: 
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@418
PS1, Line 418: e ar
> This change is according to https://www.flake8rules.com/rules/W504.html
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@633
PS1, Line 633: 
> nit: needs +4 indent instead of +2
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1023
PS1, Line 1023: 
> nit: 4 spaces are needed. Same for L1025
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1443
PS1, Line 1443: # 
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1474
PS1, Line 1474:   
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1508
PS1, Line 1508: ea
> This change is according to https://www.flake8rules.com/rules/W504.html
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1595
PS1, Line 1595: _t
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1636
PS1, Line 1636: la
> nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1994
PS1, Line 1994: la
> nit: indentation
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 18:12:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21162/3/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/21162/3/tests/query_test/test_scanners.py@1496
PS3, Line 1496: +
flake8: W504 line break after binary operator


http://gerrit.cloudera.org:8080/#/c/21162/3/tests/query_test/test_scanners.py@1498
PS3, Line 1498: +
flake8: W504 line break after binary operator



-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 18:12:48 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Joe McDonnell, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21162

to look at the new patch set (#2).

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................

IMPALA-12898: Tidy up test dimensions of test_scanner.py

This patch tidies up the test dimensions of test_scanner.py.
'exec_option' initialization is moved to add_test_dimensions() method as
much as possible. It ensures correct permutation and execution of test
cases.

After this patch, the total collected tests of test_scanner.py is 1242
for core/pairwise exploration and 7514 for exhaustive exploration.
Before, they were 794 and 11864 accordingly. The increase in test count
after refactoring with core exploration is because exec option
dimensions are now permuted correctly along with other default exec
option dimensions. The reduction in exhaustive exploration is due to a
reduction in the overall dimension to permute and a reduction in test
skipping (the test was run, but only called pytest.skip()).

Testing:
- Pass query_test/test_scanners.py in exhaustive exploration.

Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
---
M tests/query_test/test_scanners.py
1 file changed, 82 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/21162/2
-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Zoltan Borok-Nagy, Joe McDonnell, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21162

to look at the new patch set (#3).

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................

IMPALA-12898: Tidy up test dimensions of test_scanner.py

This patch tidies up the test dimensions of test_scanner.py.
'exec_option' initialization is moved to add_test_dimensions() method as
much as possible. It ensures correct permutation and execution of test
cases.

After this patch, the total collected tests of test_scanner.py is 1242
for core/pairwise exploration and 7514 for exhaustive exploration.
Before, they were 794 and 11864 accordingly. The increase in test count
after refactoring with core exploration is because exec option
dimensions are now permuted correctly along with other default exec
option dimensions. The reduction in exhaustive exploration is due to a
reduction in the overall dimension to permute and a reduction in test
skipping (the test was run, but only called pytest.skip()).

Testing:
- Pass query_test/test_scanners.py in exhaustive exploration.

Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
---
M tests/query_test/test_scanners.py
1 file changed, 82 insertions(+), 83 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/21162/3
-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/10410/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2024 13:19:20 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@74
PS1, Line 74: return [0, 1]
> Earlier [0, 1, 4] was used in core. Do you think it's not a problem to decr
I think this is OK in core, because exhaustive tests still permute with larger superset.
But I understand some test like added by IMPALA-9655 specifically want to test with medium MT_DOP size.
I'll change this and batch sizes back in next patch set. This will raise test items for core to 1242.


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@418
PS1, Line 418: and 
> nit: The original lines were more aligned, so I'm not sure if that formatti
This change is according to https://www.flake8rules.com/rules/W504.html
I don't mind changing this back since this just a warning.


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1508
PS1, Line 1508: + 
> nit: originally the lines were more aligned, so I'm not sure about this cha
This change is according to https://www.flake8rules.com/rules/W504.html
I don't mind changing this back since this just a warning.



-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 17:36:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15610/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 18:32:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15556/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Mar 2024 05:30:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 1:

This is a precursor to IMPALA-12922.


-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Mar 2024 05:30:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 1:

(17 comments)

Thanks for working on this! Mostly found style issues

http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@74
PS1, Line 74: return [0, 1]
Earlier [0, 1, 4] was used in core. Do you think it's not a problem to decrease the values in that dimension?


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@81
PS1, Line 81: return [0, 1]
Same as above, earlier it was [0, 1, 16] in core.


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@100
PS1, Line 100:   
nit: 4 spaces are needed


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@157
PS1, Line 157:   
nit: 4 spaces are needed


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@201
PS1, Line 201:   
nit: indentation is off


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@203
PS1, Line 203:   
nit: indentation


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@259
PS1, Line 259:   
nit: indentation


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@353
PS1, Line 353:   
nit: indentation


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@418
PS1, Line 418: and 
nit: The original lines were more aligned, so I'm not sure if that formatting change is beneficial


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@633
PS1, Line 633:   
nit: needs +4 indent instead of +2


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1023
PS1, Line 1023:   
nit: 4 spaces are needed. Same for L1025


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1443
PS1, Line 1443:   
nit: indentation


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1474
PS1, Line 1474:   
nit: indentation


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1508
PS1, Line 1508: + 
nit: originally the lines were more aligned, so I'm not sure about this change in formatting. Is it enforced by PEP8?


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1595
PS1, Line 1595:   
nit: indentation


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1636
PS1, Line 1636:   
nit: indentation


http://gerrit.cloudera.org:8080/#/c/21162/1/tests/query_test/test_scanners.py@1994
PS1, Line 1994:   
nit: indentation



-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 17:21:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 4: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Mar 2024 13:19:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/15611/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 18:35:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21162/2/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

http://gerrit.cloudera.org:8080/#/c/21162/2/tests/query_test/test_scanners.py@1496
PS2, Line 1496: +
flake8: W504 line break after binary operator


http://gerrit.cloudera.org:8080/#/c/21162/2/tests/query_test/test_scanners.py@1498
PS2, Line 1498: +
flake8: W504 line break after binary operator



-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Mar 2024 18:09:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-12898: Tidy up test dimensions of test scanner.py

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/21162 )

Change subject: IMPALA-12898: Tidy up test dimensions of test_scanner.py
......................................................................

IMPALA-12898: Tidy up test dimensions of test_scanner.py

This patch tidies up the test dimensions of test_scanner.py.
'exec_option' initialization is moved to add_test_dimensions() method as
much as possible. It ensures correct permutation and execution of test
cases.

After this patch, the total collected tests of test_scanner.py is 1242
for core/pairwise exploration and 7514 for exhaustive exploration.
Before, they were 794 and 11864 accordingly. The increase in test count
after refactoring with core exploration is because exec option
dimensions are now permuted correctly along with other default exec
option dimensions. The reduction in exhaustive exploration is due to a
reduction in the overall dimension to permute and a reduction in test
skipping (the test was run, but only called pytest.skip()).

Testing:
- Pass query_test/test_scanners.py in exhaustive exploration.

Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Reviewed-on: http://gerrit.cloudera.org:8080/21162
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/query_test/test_scanners.py
1 file changed, 82 insertions(+), 83 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/21162
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5efd2b483338fb55b958d8e1a0acf6b365f8093e
Gerrit-Change-Number: 21162
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>