You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2018/08/29 01:17:43 UTC
[kudu-CR] build: fix run-test.sh when not retrying all tests
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11348
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
build: fix run-test.sh when not retrying all tests
This is a follow-up to 5d69deb36925113796cd69f51061b8396b0174fc to clean
up some bash syntax. Notably, if KUDU_RETRY_ALL_FAILED_TESTS is unset,
the following message would be logged to stderr:
line 77: [: : integer expression expected
Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
---
M build-support/run-test.sh
1 file changed, 10 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/11348/1
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11348 )
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11348/3/build-support/run-test.sh
File build-support/run-test.sh:
http://gerrit.cloudera.org:8080/#/c/11348/3/build-support/run-test.sh@80
PS3, Line 80: if [ -f "$KUDU_FLAKY_TEST_LIST" ] && [ $(grep --count --line-regexp "$SHORT_TEST_NAME" "$KUDU_FLAKY_TEST_LIST") -ne 0 ]; then
I think you can simplify further:
if [ -f "$KUDU_FLAKY_TEST_LIST" ] && grep -q --line-regexp "$SHORT_TEST_NAME" "$KUDU_FLAKY_TEST_LIST"; then
...
fi
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 29 Aug 2018 20:21:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11348 )
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
Patch Set 4: Code-Review+2
If you haven't already, could you manually test the path that runs through that grep statement just to make sure it works properly? If for some reason it always fails, we'll just not retry some tests and may not notice.
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 29 Aug 2018 20:43:30 +0000
Gerrit-HasComments: No
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11348
to look at the new patch set (#4).
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
build: fix run-test.sh when not retrying all tests
This is a follow-up to 5d69deb36925113796cd69f51061b8396b0174fc to clean
up some bash syntax. Notably, if KUDU_RETRY_ALL_FAILED_TESTS is unset,
the following message would be logged to stderr:
line 77: [: : integer expression expected
Here is a dist-test run with KUDU_RETRY_ALL_FAILED_TESTS unset:
http://dist-test.cloudera.org/job?job_id=awong.1535506444.68746#
...and here is one with it set:
http://dist-test.cloudera.org/job?job_id=awong.1535506280.65606
Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
---
M build-support/run-test.sh
1 file changed, 11 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/11348/4
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11348
to look at the new patch set (#2).
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
build: fix run-test.sh when not retrying all tests
This is a follow-up to 5d69deb36925113796cd69f51061b8396b0174fc to clean
up some bash syntax. Notably, if KUDU_RETRY_ALL_FAILED_TESTS is unset,
the following message would be logged to stderr:
line 77: [: : integer expression expected
Here is a dist-test run with KUDU_RETRY_ALL_FAILED_TESTS unset:
http://dist-test.cloudera.org/job?job_id=awong.1535506444.68746#
...and here is one with it set:
http://dist-test.cloudera.org/job?job_id=awong.1535506280.65606
Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
---
M build-support/run-test.sh
1 file changed, 11 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/48/11348/2
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11348 )
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
Patch Set 2:
Also for reference, one with KUDU_RETRY_ALL_FAILED_TESTS unset without the patch: http://dist-test.cloudera.org/job?job_id=awong.1535506743.70270
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 29 Aug 2018 01:43:02 +0000
Gerrit-HasComments: No
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11348 )
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
Patch Set 4:
> Patch Set 4: Code-Review+2
>
> If you haven't already, could you manually test the path that runs through that grep statement just to make sure it works properly? If for some reason it always fails, we'll just not retry some tests and may not notice.
Yep, tried on a centos 6.6 running with an artificial list.
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 29 Aug 2018 22:22:43 +0000
Gerrit-HasComments: No
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11348 )
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11348/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11348/2//COMMIT_MSG@13
PS2, Line 13: line 77: [: : integer expression expected
Was this an actual failure? Or just a warning?
http://gerrit.cloudera.org:8080/#/c/11348/2/build-support/run-test.sh
File build-support/run-test.sh:
http://gerrit.cloudera.org:8080/#/c/11348/2/build-support/run-test.sh@80
PS2, Line 80: if [ -f "$KUDU_FLAKY_TEST_LIST" ] && [ $(grep --count --line-regexp "$SHORT_TEST_NAME" "$KUDU_FLAKY_TEST_LIST") -ne 0 ]; then
We don’t actually use the grep result; could we drop it, add -q, and take the grep out of the subshell? That is, just use the grep exit status directly in the condition?
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 29 Aug 2018 06:11:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11348 )
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11348/3/build-support/run-test.sh
File build-support/run-test.sh:
http://gerrit.cloudera.org:8080/#/c/11348/3/build-support/run-test.sh@80
PS3, Line 80: if [ -f "$KUDU_FLAKY_TEST_LIST" ] && grep -q --count --line-regexp "$SHORT_TEST_NAME" "$KUDU_FLAKY_TEST_LIST"; then
> I think you can simplify further:
Doh, didn't add the changes. Done
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 29 Aug 2018 20:34:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11348 )
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11348/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11348/2//COMMIT_MSG@13
PS2, Line 13: line 77: [: : integer expression expected
> Was this an actual failure? Or just a warning?
I think it was a log that didn't necessarily fail since we're still reporting failures to the dashboard, so the rest of run-test.sh must be running. That said, this may have broken retries.
http://gerrit.cloudera.org:8080/#/c/11348/2/build-support/run-test.sh
File build-support/run-test.sh:
http://gerrit.cloudera.org:8080/#/c/11348/2/build-support/run-test.sh@80
PS2, Line 80: if [ -f "$KUDU_FLAKY_TEST_LIST" ] && [ $(grep --count --line-regexp "$SHORT_TEST_NAME" "$KUDU_FLAKY_TEST_LIST") -ne 0 ]; then
> We don’t actually use the grep result; could we drop it, add -q, and take t
Neat, done
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 29 Aug 2018 19:14:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] build: fix run-test.sh when not retrying all tests
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11348 )
Change subject: build: fix run-test.sh when not retrying all tests
......................................................................
build: fix run-test.sh when not retrying all tests
This is a follow-up to 5d69deb36925113796cd69f51061b8396b0174fc to clean
up some bash syntax. Notably, if KUDU_RETRY_ALL_FAILED_TESTS is unset,
the following message would be logged to stderr:
line 77: [: : integer expression expected
Here is a dist-test run with KUDU_RETRY_ALL_FAILED_TESTS unset:
http://dist-test.cloudera.org/job?job_id=awong.1535506444.68746#
...and here is one with it set:
http://dist-test.cloudera.org/job?job_id=awong.1535506280.65606
Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Reviewed-on: http://gerrit.cloudera.org:8080/11348
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M build-support/run-test.sh
1 file changed, 11 insertions(+), 13 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/11348
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icb208194fed255361b2987e945ef91779742d4d7
Gerrit-Change-Number: 11348
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins