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/09/07 06:11:37 UTC

[kudu-CR] build: add option to ignore test failures 

Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11399


Change subject: build: add option to ignore test failures
......................................................................

build: add option to ignore test failures

Sometimes when running build-and-test.sh, we may not care about the
failure of any given test, so long as the tests ran, e.g. if running
only for the sake of reporting the results of the tests. As such, this
patch adds an option to ignore test failures during runs of
build-and-test.sh.

I tested this by injecting errors into a test and doing a
build-and-test.sh run with the new ERROR_ON_TEST_FAILURE=1, verifying
that it still returns an error; and with ERROR_ON_TEST_FAILURE=0,
verifying that it returns no error. I also broke the build and verified
that, despite ERROR_ON_TEST_FAILURE=0, it returns an error.

Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
---
M build-support/jenkins/build-and-test.sh
1 file changed, 19 insertions(+), 12 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/11399/1
-- 
To view, visit http://gerrit.cloudera.org:8080/11399
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>

[kudu-CR] build: add option to ignore test failures

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/11399

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

Change subject: build: add option to ignore test failures
......................................................................

build: add option to ignore test failures

Sometimes when running build-and-test.sh, we may not care about the
failure of any given test, so long as the tests ran, e.g. if running
only for the sake of reporting the results of the tests. As such, this
patch adds an option to ignore test failures during runs of
build-and-test.sh.

I tested this by injecting errors into a test and doing a
build-and-test.sh run with the new ERROR_ON_TEST_FAILURE=1, verifying
that it still returns an error; and with ERROR_ON_TEST_FAILURE=0,
verifying that it returns no error. I also broke the build and verified
that, despite ERROR_ON_TEST_FAILURE=0, it returns an error.

Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
---
M build-support/jenkins/build-and-test.sh
1 file changed, 21 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/11399/3
-- 
To view, visit http://gerrit.cloudera.org:8080/11399
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
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

[kudu-CR] build: add option to ignore test failures

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/11399

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

Change subject: build: add option to ignore test failures
......................................................................

build: add option to ignore test failures

Sometimes when running build-and-test.sh, we may not care about the
failure of any given test, so long as the tests ran, e.g. if running
only for the sake of reporting the results of the tests. As such, this
patch adds an option to ignore test failures during runs of
build-and-test.sh.

I tested this by injecting errors into a test and doing a
build-and-test.sh run with the new ERROR_ON_TEST_FAILURE=1, verifying
that it still returns an error; and with ERROR_ON_TEST_FAILURE=0,
verifying that it returns no error. I also broke the build and verified
that, despite ERROR_ON_TEST_FAILURE=0, it returns an error.

Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
---
M build-support/jenkins/build-and-test.sh
1 file changed, 21 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/11399/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11399
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] build: add option to ignore test failures

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

Change subject: build: add option to ignore test failures
......................................................................


Patch Set 3: Code-Review+2

But you might want to get another set of eyes on this.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
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: Mon, 10 Sep 2018 23:16:11 +0000
Gerrit-HasComments: No

[kudu-CR] build: add option to ignore test failures

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

Change subject: build: add option to ignore test failures
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11399/2/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/11399/2/build-support/jenkins/build-and-test.sh@618
PS2, Line 618: if [ "$TESTS_FAILED" != "0" -o "$EXIT_STATUS" != "0" ]; then
> Could you consistently double quote (or not) TESTS_FAILED? And the operand 
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
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: Mon, 10 Sep 2018 22:58:58 +0000
Gerrit-HasComments: Yes

[kudu-CR] build: add option to ignore test failures

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

Change subject: build: add option to ignore test failures
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11399/1/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/11399/1/build-support/jenkins/build-and-test.sh@618
PS1, Line 618: if [ $TESTS_FAILED != 0 -o $EXIT_STATUS != 0 ]; then
> You can combine this:
Done


http://gerrit.cloudera.org:8080/#/c/11399/1/build-support/jenkins/build-and-test.sh@658
PS1, Line 658: # If any of the tests failed and we are honoring test failures, set the exit
> And combine this too.
Done


http://gerrit.cloudera.org:8080/#/c/11399/1/build-support/jenkins/build-and-test.sh@659
PS1, Line 659: # status to 1. Note that it may have already been set to 1 above.
> Should we avoid overwriting if EXIT_STATUS is non-zero? Is that even possib
Maybe it's an implementation detail, but EXIT_STATUS only ever gets sets to 1, and is 0 otherwise. I'll add a comment to that effect.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
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: Mon, 10 Sep 2018 21:54:05 +0000
Gerrit-HasComments: Yes

[kudu-CR] build: add option to ignore test failures

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

Change subject: build: add option to ignore test failures
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11399/2/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/11399/2/build-support/jenkins/build-and-test.sh@618
PS2, Line 618: if [ $TESTS_FAILED != 0 -o $EXIT_STATUS != 0 ]; then
Could you consistently double quote (or not) TESTS_FAILED? And the operand you're comparing it against?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
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: Mon, 10 Sep 2018 22:22:22 +0000
Gerrit-HasComments: Yes

[kudu-CR] build: add option to ignore test failures

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

Change subject: build: add option to ignore test failures
......................................................................


Patch Set 3:

Sure, tagging in Mike, since he might be the most immediately affected.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
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-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 11 Sep 2018 02:11:39 +0000
Gerrit-HasComments: No

[kudu-CR] build: add option to ignore test failures

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

Change subject: build: add option to ignore test failures
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11399/1/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

http://gerrit.cloudera.org:8080/#/c/11399/1/build-support/jenkins/build-and-test.sh@618
PS1, Line 618: if [ $TESTS_FAILED != 0 ] || [ $EXIT_STATUS != 0 ]; then
You can combine this:

  if [ <expr1> -o <expr2> ]; then
    ...
  fi


http://gerrit.cloudera.org:8080/#/c/11399/1/build-support/jenkins/build-and-test.sh@658
PS1, Line 658: if [ "$ERROR_ON_TEST_FAILURE" == "1" ] && [ "$TESTS_FAILED" == "1" ]; then
And combine this too.


http://gerrit.cloudera.org:8080/#/c/11399/1/build-support/jenkins/build-and-test.sh@659
PS1, Line 659:   EXIT_STATUS=1
Should we avoid overwriting if EXIT_STATUS is non-zero? Is that even possible?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Mon, 10 Sep 2018 20:12:41 +0000
Gerrit-HasComments: Yes

[kudu-CR] build: add option to ignore test failures

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11399 )

Change subject: build: add option to ignore test failures
......................................................................

build: add option to ignore test failures

Sometimes when running build-and-test.sh, we may not care about the
failure of any given test, so long as the tests ran, e.g. if running
only for the sake of reporting the results of the tests. As such, this
patch adds an option to ignore test failures during runs of
build-and-test.sh.

I tested this by injecting errors into a test and doing a
build-and-test.sh run with the new ERROR_ON_TEST_FAILURE=1, verifying
that it still returns an error; and with ERROR_ON_TEST_FAILURE=0,
verifying that it returns no error. I also broke the build and verified
that, despite ERROR_ON_TEST_FAILURE=0, it returns an error.

Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Reviewed-on: http://gerrit.cloudera.org:8080/11399
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M build-support/jenkins/build-and-test.sh
1 file changed, 21 insertions(+), 12 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified
  Grant Henke: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] build: add option to ignore test failures

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

Change subject: build: add option to ignore test failures
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I27ecacd499423755dbf742bb70db6a7f1e5c9db7
Gerrit-Change-Number: 11399
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Tue, 11 Sep 2018 14:06:53 +0000
Gerrit-HasComments: No