You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2017/10/09 22:27:18 UTC
[Impala-ASF-CR] IMPALA-6023: Fix broken breakpad test
Lars Volker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8240
Change subject: IMPALA-6023: Fix broken breakpad test
......................................................................
IMPALA-6023: Fix broken breakpad test
We have a test to make sure that hitting a DCHECK will write a minidump.
We used to pass "-beeswax_port=1" to the server to trigger a DCHECK. A
while ago, this DCHECK seems to have been removed, but we still called
abort() if the ImpalaServer failed to start. This masked the slightly
altered behavior and the test still succeeded.
However, the fix for IMPALA-4786 changed the behavior to call exit(1)
instead of abort() if the ImpalaServer failed to start.
To fix the test, we change it to pass an unresolvable hostname to
impalad, which will result in a call to abort().
This change also splits the breakpad tests into core and exhaustive sets
to make sure that tests which depend on other parts of Impala are
included in every core run.
Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 35 insertions(+), 22 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/8240/3
--
To view, visit http://gerrit.cloudera.org:8080/8240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Gerrit-Change-Number: 8240
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-6023: Fix broken breakpad test
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8240 )
Change subject: IMPALA-6023: Fix broken breakpad test
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1325/
--
To view, visit http://gerrit.cloudera.org:8080/8240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Gerrit-Change-Number: 8240
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Oct 2017 19:06:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6023: Fix broken breakpad test
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8240 )
Change subject: IMPALA-6023: Fix broken breakpad test
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8240/3/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:
http://gerrit.cloudera.org:8080/#/c/8240/3/tests/custom_cluster/test_breakpad.py@149
PS3, Line 149: @pytest.mark.execute_serially
> This was here due to needing DCHECK behavior before, right? What about abor
Done
http://gerrit.cloudera.org:8080/#/c/8240/3/tests/custom_cluster/test_breakpad.py@174
PS3, Line 174:
> Change to:
Done
http://gerrit.cloudera.org:8080/#/c/8240/3/tests/custom_cluster/test_breakpad.py@333
PS3, Line 333:
> Some blank lines at the end of the file.
Done
--
To view, visit http://gerrit.cloudera.org:8080/8240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Gerrit-Change-Number: 8240
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Oct 2017 17:26:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6023: Fix broken breakpad test
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/8240 )
Change subject: IMPALA-6023: Fix broken breakpad test
......................................................................
IMPALA-6023: Fix broken breakpad test
We have a test to make sure that hitting a DCHECK will write a minidump.
We used to pass "-beeswax_port=1" to the server to trigger a DCHECK. A
while ago, this DCHECK seems to have been removed, but we still called
abort() if the ImpalaServer failed to start. This masked the slightly
altered behavior and the test still succeeded.
However, the fix for IMPALA-4786 changed the behavior to call exit(1)
instead of abort() if the ImpalaServer failed to start.
To fix the test, we change it to pass an unresolvable hostname to
impalad, which will result in a call to abort().
This change also splits the breakpad tests into core and exhaustive sets
to make sure that tests which depend on other parts of Impala are
included in every core run.
Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Reviewed-on: http://gerrit.cloudera.org:8080/8240
Reviewed-by: Michael Brown <mi...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 33 insertions(+), 23 deletions(-)
Approvals:
Michael Brown: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/8240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Gerrit-Change-Number: 8240
Gerrit-PatchSet: 5
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-6023: Fix broken breakpad test
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8240 )
Change subject: IMPALA-6023: Fix broken breakpad test
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Gerrit-Change-Number: 8240
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Oct 2017 23:08:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6023: Fix broken breakpad test
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Sailesh Mukil,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8240
to look at the new patch set (#4).
Change subject: IMPALA-6023: Fix broken breakpad test
......................................................................
IMPALA-6023: Fix broken breakpad test
We have a test to make sure that hitting a DCHECK will write a minidump.
We used to pass "-beeswax_port=1" to the server to trigger a DCHECK. A
while ago, this DCHECK seems to have been removed, but we still called
abort() if the ImpalaServer failed to start. This masked the slightly
altered behavior and the test still succeeded.
However, the fix for IMPALA-4786 changed the behavior to call exit(1)
instead of abort() if the ImpalaServer failed to start.
To fix the test, we change it to pass an unresolvable hostname to
impalad, which will result in a call to abort().
This change also splits the breakpad tests into core and exhaustive sets
to make sure that tests which depend on other parts of Impala are
included in every core run.
Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
---
M tests/custom_cluster/test_breakpad.py
1 file changed, 33 insertions(+), 23 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/8240/4
--
To view, visit http://gerrit.cloudera.org:8080/8240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Gerrit-Change-Number: 8240
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
[Impala-ASF-CR] IMPALA-6023: Fix broken breakpad test
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/8240 )
Change subject: IMPALA-6023: Fix broken breakpad test
......................................................................
Patch Set 3:
(3 comments)
Thanks for looking into this.
http://gerrit.cloudera.org:8080/#/c/8240/3/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:
http://gerrit.cloudera.org:8080/#/c/8240/3/tests/custom_cluster/test_breakpad.py@149
PS3, Line 149: @SkipIfBuildType.not_dev_build
This was here due to needing DCHECK behavior before, right? What about abort() on release builds? Can this be removed?
http://gerrit.cloudera.org:8080/#/c/8240/3/tests/custom_cluster/test_breakpad.py@174
PS3, Line 174: super(TestBreakpadExhaustive).setup_class();
Change to:
super(TestBreakpadExhaustive, cls).setup_class()
1. Drop the semi-colon
2. Pass in the cls argument to super()
Although what you wrote is syntactically valid, it's semantically invalid in this context and fails at runtime. More here: https://docs.python.org/2.7/library/functions.html#super
Btw, I found this doing an accounting of your tests to be run in both core and exhaustive:
impala-py.test --workload_exploration_strategy functional-query:exhaustive tests/custom_cluster/test_breakpad.py
http://gerrit.cloudera.org:8080/#/c/8240/3/tests/custom_cluster/test_breakpad.py@333
PS3, Line 333:
Some blank lines at the end of the file.
--
To view, visit http://gerrit.cloudera.org:8080/8240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Gerrit-Change-Number: 8240
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Oct 2017 15:50:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6023: Fix broken breakpad test
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/8240 )
Change subject: IMPALA-6023: Fix broken breakpad test
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/8240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb5af3e72963280a6677a99aa6a0e5785443bb0c
Gerrit-Change-Number: 8240
Gerrit-PatchSet: 4
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Oct 2017 17:47:56 +0000
Gerrit-HasComments: No