You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org> on 2018/04/12 23:16:02 UTC
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10053
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
IMPALA-6845: TestHdfsQueries causes some tests to be run twice
TestHdfsQueries is a subclass of TestQueries and inherits of all its 'test_*' methods,
causing these tests to be run twice any time test_queries.py is run. This was not
intentional (it was subclassed just to inherit 'add_test_dimensions') and causes test runs
to take longer than necessary.
This patch removes the subclass relationship and copies the logic in add_test_dimensions()
from TestQueries in HdfsTestQueries, with a convenience function added to minimize code
duplication.
Testing:
- Ran test_queries.py under both 'core' and 'exhaustive' and checked that the same tests
are run, except all now only a single time each.
Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
---
M tests/common/test_dimensions.py
M tests/query_test/test_queries.py
2 files changed, 28 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/10053/1
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
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/10053 )
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
IMPALA-6845: TestHdfsQueries causes some tests to be run twice
TestHdfsQueries is a subclass of TestQueries and inherits of all its
'test_*' methods, causing these tests to be run twice any time
test_queries.py is run. This was not intentional (it was subclassed
just to inherit 'add_test_dimensions') and causes test runs to take
longer than necessary.
This patch removes the subclass relationship and copies the logic in
add_test_dimensions() from TestQueries in HdfsTestQueries, with a
convenience function added to minimize code duplication.
Testing:
- Ran test_queries.py under both 'core' and 'exhaustive' and checked
that the same tests are run, except all now only a single time each.
Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Reviewed-on: http://gerrit.cloudera.org:8080/10053
Reviewed-by: Michael Brown <mi...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/common/test_dimensions.py
M tests/query_test/test_queries.py
2 files changed, 28 insertions(+), 16 deletions(-)
Approvals:
Michael Brown: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/10053 )
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Apr 2018 17:40:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/10053 )
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10053/2/tests/common/test_dimensions.py
File tests/common/test_dimensions.py:
http://gerrit.cloudera.org:8080/#/c/10053/2/tests/common/test_dimensions.py@193
PS2, Line 193: new_value[-1].value[key] = value
> copy/paste. I think here's where you want to use key and value
Done
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Apr 2018 17:22:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/10053 )
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
IMPALA-6845: TestHdfsQueries causes some tests to be run twice
TestHdfsQueries is a subclass of TestQueries and inherits of all its
'test_*' methods, causing these tests to be run twice any time
test_queries.py is run. This was not intentional (it was subclassed
just to inherit 'add_test_dimensions') and causes test runs to take
longer than necessary.
This patch removes the subclass relationship and copies the logic in
add_test_dimensions() from TestQueries in HdfsTestQueries, with a
convenience function added to minimize code duplication.
Testing:
- Ran test_queries.py under both 'core' and 'exhaustive' and checked
that the same tests are run, except all now only a single time each.
Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
---
M tests/common/test_dimensions.py
M tests/query_test/test_queries.py
2 files changed, 28 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/10053/2
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10053 )
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
Patch Set 4:
Nice find!
If you add something like this to tests/conftest.py, we'll be able to prevent one of these sneaking up on us. What this is saying is that a given code location for a test function should only appear as coming from a single class. I found with impala-py.test tests/query_test/ --collect-only -s --pdb that this captured your case here. (And, the good news is I don't think we have other cases like it!)
@pytest.hookimpl(trylast=True)
def pytest_collection_modifyitems(items, config, session):
+ # Find tests that show up via subclassing
+ # ('tests/query_test/test_queries.py', 118, "TestHdfsQueries.test_empty[exec_option: {'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 'abort_on_error': 1, 'exec_single_node_rows_threshold': 0} | table_f ormat: parquet/none]")
+ klass_by_location = dict()
+ for i in items:
+ a, b, name = i.location
+ klass = name.split(".")[0]
+ loc = (a,b)
+ if loc in klass_by_location:
+ assert klass == klass_by_location[loc]
+ else:
+ klass_by_location[loc] = klass
+
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Apr 2018 22:52:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/10053 )
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
Patch Set 2:
(1 comment)
great catch
http://gerrit.cloudera.org:8080/#/c/10053/2/tests/common/test_dimensions.py
File tests/common/test_dimensions.py:
http://gerrit.cloudera.org:8080/#/c/10053/2/tests/common/test_dimensions.py@193
PS2, Line 193: new_value[-1].value["exec_single_node_rows_threshold"] = 100
copy/paste. I think here's where you want to use key and value
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Apr 2018 23:26:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10053 )
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2307/
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Apr 2018 17:56:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10053 )
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 13 Apr 2018 21:46:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6845: TestHdfsQueries causes some tests to be run twice
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10053
to look at the new patch set (#3).
Change subject: IMPALA-6845: TestHdfsQueries causes some tests to be run twice
......................................................................
IMPALA-6845: TestHdfsQueries causes some tests to be run twice
TestHdfsQueries is a subclass of TestQueries and inherits of all its
'test_*' methods, causing these tests to be run twice any time
test_queries.py is run. This was not intentional (it was subclassed
just to inherit 'add_test_dimensions') and causes test runs to take
longer than necessary.
This patch removes the subclass relationship and copies the logic in
add_test_dimensions() from TestQueries in HdfsTestQueries, with a
convenience function added to minimize code duplication.
Testing:
- Ran test_queries.py under both 'core' and 'exhaustive' and checked
that the same tests are run, except all now only a single time each.
Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
---
M tests/common/test_dimensions.py
M tests/query_test/test_queries.py
2 files changed, 28 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/10053/3
--
To view, visit http://gerrit.cloudera.org:8080/10053
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ida659aa7b5131a6a7469baa93a41f7581bd0659a
Gerrit-Change-Number: 10053
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>