You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2019/03/29 00:54:29 UTC

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12885


Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................

IMPALA-8371: Return appropriate error code for unified backend tests

Unified backend tests rely on generating bash scripts for each test
that call the unified executable with a filter to run the appropriate
subset of the tests. The generated script currently does not return
the return code from the test execution.

This changes the test execution scripts to return the appropriate
return code. To do this, the script generator is changed from
a bash implementation in bin/gen-backend-test-script.sh to
a python implementation in bin/gen-backend-test-script.py.
This makes it easier to handle shell variables in the script
template correctly.

Testing:
 - Ran backend tests on centos 6, centos 7
 - Manually tested with a failing test and verified return value

Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
---
M be/CMakeLists.txt
A bin/gen-backend-test-script.py
D bin/gen-backend-test-script.sh
3 files changed, 85 insertions(+), 51 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................

IMPALA-8371: Return appropriate error code for unified backend tests

Unified backend tests rely on generating bash scripts for each test
that call the unified executable with a filter to run the appropriate
subset of the tests. The generated script currently does not return
the return code from the test execution.

This changes the test execution scripts to return the appropriate
return code. To do this, the script generator is changed from
a bash implementation in bin/gen-backend-test-script.sh to
a python implementation in bin/gen-backend-test-script.py.
This makes it easier to handle shell variables in the script
template correctly.

Testing:
 - Ran backend tests on centos 6, centos 7
 - Manually tested with a failing test and verified return value

Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
---
M be/CMakeLists.txt
A bin/gen-backend-test-script.py
D bin/gen-backend-test-script.sh
3 files changed, 80 insertions(+), 51 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/12885/6
-- 
To view, visit http://gerrit.cloudera.org:8080/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2664/ : 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/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Sat, 06 Apr 2019 00:05:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12885/4/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/4/bin/gen-backend-test-script.py@29
PS4, Line 29: import subprocess
flake8: F401 'subprocess' imported but unused


http://gerrit.cloudera.org:8080/#/c/12885/4/bin/gen-backend-test-script.py@30
PS4, Line 30: import sys
flake8: F401 'sys' imported but unused



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2019 23:32:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 2:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@20
PS2, Line 20: take
> nit: spelling
Done


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@26
PS2, Line 26: from optparse import OptionParser
> optparse has been deprecated since python 2.7, use argparse instead?
Switched to argparse


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@37
PS2, Line 37: """).lstrip()
> Is the lstrip needed?
It gets rid of a leading blank line. Added a comment about it.


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@41
PS2, Line 41: """
> If you prefix this with an r"""...""" you get a literal string and don't ne
That's good to know! Switched over and removed the escaping.


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@63
PS2, Line 63:     parser = OptionParser()
> indent 2 spaces
Done


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@67
PS2, Line 67:     if options.gtest_filter is None or options.test_script_output is None:
> You can mark options as required in argparse
Done


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@77
PS2, Line 77:     subprocess.check_call(["chmod", "+x", options.test_script_output])
> Could also use os.chmod()
Switched over


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@80
PS2, Line 80: if __name__ == "__main__": main()
> nit: we usually would put a newline after the :
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2019 23:31:43 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................

IMPALA-8371: Return appropriate error code for unified backend tests

Unified backend tests rely on generating bash scripts for each test
that call the unified executable with a filter to run the appropriate
subset of the tests. The generated script currently does not return
the return code from the test execution.

This changes the test execution scripts to return the appropriate
return code. To do this, the script generator is changed from
a bash implementation in bin/gen-backend-test-script.sh to
a python implementation in bin/gen-backend-test-script.py.
This makes it easier to handle shell variables in the script
template correctly.

Testing:
 - Ran backend tests on centos 6, centos 7
 - Manually tested with a failing test and verified return value

Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
---
M be/CMakeLists.txt
A bin/gen-backend-test-script.py
D bin/gen-backend-test-script.sh
3 files changed, 83 insertions(+), 51 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 2: Code-Review+1

(2 comments)

LGTM

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@21
PS2, Line 21: # 1: The file location to write the generated script
Aren't these parameters now named (--gtest_filter,--test_script_output) rather than positional?


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@32
PS2, Line 32: # The script template requires substitutions to set the variables used by the body.
Nit: Clearer maybe to say "the script header template".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 16:38:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@37
PS2, Line 37: """).lstrip()
> I see. If you add a "\" at the end of the first line, it'll dedent the rest
Good point, switched to use the "\" and removed the lstrip


http://gerrit.cloudera.org:8080/#/c/12885/5/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/5/bin/gen-backend-test-script.py@24
PS5, Line 24: # This script is used by the build system and is not intended to be run directly.
> I'd be inclined to drop the parameter description above and repeat them in 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:46:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................

IMPALA-8371: Return appropriate error code for unified backend tests

Unified backend tests rely on generating bash scripts for each test
that call the unified executable with a filter to run the appropriate
subset of the tests. The generated script currently does not return
the return code from the test execution.

This changes the test execution scripts to return the appropriate
return code. To do this, the script generator is changed from
a bash implementation in bin/gen-backend-test-script.sh to
a python implementation in bin/gen-backend-test-script.py.
This makes it easier to handle shell variables in the script
template correctly.

Testing:
 - Ran backend tests on centos 6, centos 7
 - Manually tested with a failing test and verified return value

Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
---
M be/CMakeLists.txt
A bin/gen-backend-test-script.py
D bin/gen-backend-test-script.sh
3 files changed, 83 insertions(+), 51 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 3: Code-Review+1

Carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:52:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2665/ : 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/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Sat, 06 Apr 2019 00:17:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 6:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:47:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................

IMPALA-8371: Return appropriate error code for unified backend tests

Unified backend tests rely on generating bash scripts for each test
that call the unified executable with a filter to run the appropriate
subset of the tests. The generated script currently does not return
the return code from the test execution.

This changes the test execution scripts to return the appropriate
return code. To do this, the script generator is changed from
a bash implementation in bin/gen-backend-test-script.sh to
a python implementation in bin/gen-backend-test-script.py.
This makes it easier to handle shell variables in the script
template correctly.

Testing:
 - Ran backend tests on centos 6, centos 7
 - Manually tested with a failing test and verified return value

Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
---
M be/CMakeLists.txt
A bin/gen-backend-test-script.py
D bin/gen-backend-test-script.sh
3 files changed, 82 insertions(+), 51 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/12885/5
-- 
To view, visit http://gerrit.cloudera.org:8080/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12885/4/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/4/bin/gen-backend-test-script.py@29
PS4, Line 29: import subprocess
> flake8: F401 'subprocess' imported but unused
Done


http://gerrit.cloudera.org:8080/#/c/12885/4/bin/gen-backend-test-script.py@30
PS4, Line 30: import sys
> flake8: F401 'sys' imported but unused
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2019 23:38:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 1:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@25
PS1, Line 25: #TEST_EXEC_LOCATION=${1}
> flake8: E265 block comment should start with '# '
Done


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@26
PS1, Line 26: #GTEST_FILTER=${2}
> flake8: E265 block comment should start with '# '
Done


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@27
PS1, Line 27: #TEST_EXEC_NAME=$(basename "${TEST_EXEC_LOCATION}")
> flake8: E265 block comment should start with '# '
Done


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@36
PS1, Line 36: """#!/bin/bash
> flake8: E122 continuation line missing indentation or outdented
Done


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@44
PS1, Line 44: """
> flake8: E122 continuation line missing indentation or outdented
Done


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@64
PS1, Line 64: def main():
> flake8: E302 expected 2 blank lines, found 1
Done


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@71
PS1, Line 71: ;
> flake8: E703 statement ends with a semicolon
Done


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@81
PS1, Line 81: if __name__ == "__main__": main()
> flake8: E305 expected 2 blank lines after class or function definition, fou
Done


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@82
PS1, Line 82: 
> flake8: W391 blank line at end of file
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Mar 2019 01:19:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2585/ : 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/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Mar 2019 01:39:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@21
PS2, Line 21: # 1: The file location to write the generated script
> Aren't these parameters now named (--gtest_filter,--test_script_output) rat
Good point, I updated this comment to reflect the new args.


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@32
PS2, Line 32: # The script template requires substitutions to set the variables used by the body.
> Nit: Clearer maybe to say "the script header template".
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 22:51:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 5: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@37
PS2, Line 37: """).lstrip()
> It gets rid of a leading blank line. Added a comment about it.
I see. If you add a "\" at the end of the first line, it'll dedent the rest properly and you shouldn't have a blank line (that seems to be the suggestion of the dedent documentation).


http://gerrit.cloudera.org:8080/#/c/12885/5/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/5/bin/gen-backend-test-script.py@24
PS5, Line 24: # This script is used by the build system and is not intended to be run directly.
I'd be inclined to drop the parameter description above and repeat them in the --help section below. The script is not intended to be called manually anyways. Maybe replace with a comment like "for an example usage, see other_file.sh"?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 5
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 16:41:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Lars Volker, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................

IMPALA-8371: Return appropriate error code for unified backend tests

Unified backend tests rely on generating bash scripts for each test
that call the unified executable with a filter to run the appropriate
subset of the tests. The generated script currently does not return
the return code from the test execution.

This changes the test execution scripts to return the appropriate
return code. To do this, the script generator is changed from
a bash implementation in bin/gen-backend-test-script.sh to
a python implementation in bin/gen-backend-test-script.py.
This makes it easier to handle shell variables in the script
template correctly.

Testing:
 - Ran backend tests on centos 6, centos 7
 - Manually tested with a failing test and verified return value

Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
---
M be/CMakeLists.txt
A bin/gen-backend-test-script.py
D bin/gen-backend-test-script.sh
3 files changed, 84 insertions(+), 51 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/12885/4
-- 
To view, visit http://gerrit.cloudera.org:8080/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 4
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Apr 2019 01:38:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 3:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@20
PS2, Line 20: take
nit: spelling


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@26
PS2, Line 26: from optparse import OptionParser
optparse has been deprecated since python 2.7, use argparse instead?


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@37
PS2, Line 37: """).lstrip()
Is the lstrip needed?


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@41
PS2, Line 41: """
If you prefix this with an r"""...""" you get a literal string and don't need to escape the backslashes


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@63
PS2, Line 63:     parser = OptionParser()
indent 2 spaces


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@67
PS2, Line 67:     if options.gtest_filter is None or options.test_script_output is None:
You can mark options as required in argparse


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@77
PS2, Line 77:     subprocess.check_call(["chmod", "+x", options.test_script_output])
Could also use os.chmod()


http://gerrit.cloudera.org:8080/#/c/12885/2/bin/gen-backend-test-script.py@80
PS2, Line 80: if __name__ == "__main__": main()
nit: we usually would put a newline after the :



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Fri, 05 Apr 2019 17:24:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2616/ : 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/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 3
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Apr 2019 23:31:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................

IMPALA-8371: Return appropriate error code for unified backend tests

Unified backend tests rely on generating bash scripts for each test
that call the unified executable with a filter to run the appropriate
subset of the tests. The generated script currently does not return
the return code from the test execution.

This changes the test execution scripts to return the appropriate
return code. To do this, the script generator is changed from
a bash implementation in bin/gen-backend-test-script.sh to
a python implementation in bin/gen-backend-test-script.py.
This makes it easier to handle shell variables in the script
template correctly.

Testing:
 - Ran backend tests on centos 6, centos 7
 - Manually tested with a failing test and verified return value

Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Reviewed-on: http://gerrit.cloudera.org:8080/12885
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/CMakeLists.txt
A bin/gen-backend-test-script.py
D bin/gen-backend-test-script.sh
3 files changed, 80 insertions(+), 51 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 7
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2681/ : 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/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 21:10:59 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 6: Code-Review+2

Carry +2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 6
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Apr 2019 20:46:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 1:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py
File bin/gen-backend-test-script.py:

http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@25
PS1, Line 25: #TEST_EXEC_LOCATION=${1}
flake8: E265 block comment should start with '# '


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@26
PS1, Line 26: #GTEST_FILTER=${2}
flake8: E265 block comment should start with '# '


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@27
PS1, Line 27: #TEST_EXEC_NAME=$(basename "${TEST_EXEC_LOCATION}")
flake8: E265 block comment should start with '# '


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@36
PS1, Line 36: """#!/bin/bash
flake8: E122 continuation line missing indentation or outdented


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@44
PS1, Line 44: """
flake8: E122 continuation line missing indentation or outdented


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@64
PS1, Line 64: def main():
flake8: E302 expected 2 blank lines, found 1


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@71
PS1, Line 71: ;
flake8: E703 statement ends with a semicolon


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@81
PS1, Line 81: if __name__ == "__main__": main()
flake8: E305 expected 2 blank lines after class or function definition, found 1


http://gerrit.cloudera.org:8080/#/c/12885/1/bin/gen-backend-test-script.py@82
PS1, Line 82: 
flake8: W391 blank line at end of file



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Mar 2019 00:55:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8371: Return appropriate error code for unified backend tests

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

Change subject: IMPALA-8371: Return appropriate error code for unified backend tests
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2584/ : 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/12885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia146d026d42f76d5ea12d92798f299182de03eef
Gerrit-Change-Number: 12885
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Mar 2019 01:38:51 +0000
Gerrit-HasComments: No