David Knupp has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9494 )

Change subject: IMPALA-5886 & IMPALA-4812 Update run-tests.py to handle 
exit_code 5
......................................................................


Patch Set 12:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9494/11/tests/run-tests.py
File tests/run-tests.py:

http://gerrit.cloudera.org:8080/#/c/9494/11/tests/run-tests.py@112
PS11, Line 112: d on the outcome of test run when no tests
> Patch Set 12:
>
> (7 comments)

Since you didn't include your command line, it's not clear what tests you were 
running, so I'm not sure what to make of your debug output. But here's an 
illustration of what I was trying to get at.

I've added a dummy test that simply asserts True (called "test_always_passes") 
to one of our test modules. Because I didn't mark it as a serial test, 
run-tests.py should find this test when it tries to run the parallel tests.

BEFORE your patch, when I try to run just this test in isolation, run-tests.py 
finds nothing in the serial tests, stress, or custom cluster tests (as 
expected), but it does successfully find and execute the test in the parallel 
test run.

  $ impala-python run-tests.py shell/test_shell_commandline.py -k 
test_always_passes
  ==================== test session starts =====================
  [...]
  scheduling tests via LoadScheduling

  shell/test_shell_commandline.py::test_always_passes
  [gw1] PASSED shell/test_shell_commandline.py::test_always_passes
  generated xml file: /Impala/logs/ee_tests/results/TEST-impala-parallel.xml
  ================== 1 passed in 1.22 seconds ==================


When I query the run-tests.py exit code, it should be zero, however:

  systest@impala-tc-u1404-client:~/Impala/tests$ echo $?
  1  # <---- This is a bug. Everything ran as expected, and passed.


For a second use case, I'll try to run a test that I know is NOT a valid test.

  $ impala-python run-tests.py shell/test_shell_commandline.py -k 
test_does_not_exist
  [...no tests are found in any group, obviously...]


Now when I query the exit code, it should show that there was an error, and it 
does.

  systest@impala-tc-u1404-client:~/Impala/tests$ echo $?
  1  # <-------- This is correct. We tried a non-existent test, so we should 
exit with an error.


AFTER your patch, when I try the same experiment, I  can see that the first 
issue has indeed been corrected.

  $ impala-python run-tests.py shell/test_shell_commandline.py -k 
test_always_passes
  ==================== test session starts =====================
  [...]
  scheduling tests via LoadScheduling

  shell/test_shell_commandline.py::test_always_passes
  [gw1] PASSED shell/test_shell_commandline.py::test_always_passes
    generated xml file: /Impala/logs/ee_tests/results/TEST-impala-parallel.xml
  ================== 1 passed in 1.22 seconds ==================
  systest@impala-tc-u1404-client:~/Impala/tests$ echo $?
  0  # <---- This is correct. We found the test, ran it, and it passed.


However, now there's a problem in the second use case. If I try to run an 
invalid test, run-tests.py tells me that everything is OK.

  $ impala-python run-tests.py shell/test_shell_commandline.py -k 
test_does_not_exist
  [...no tests are found in any group, obviously...]
  ================ no tests ran in 1.22 seconds ================
  systest@impala-tc-u1404-client:~/Impala/tests$ echo $?
  0  # <---- This is a new bug. Exit code should not be zero in this case.


This is also what I was trying to get at with the earlier suggestion that the 
pytest exit code and the overall run-tests.py exit code are different things. 
The pytest exit status is evaluated after each invocation of pytest. The 
overall run-tests.py exit code can only be evaluated at the very end, after 
we've gone through all of our pytest attempts.

I'm sorry that I didn't make this more clear before.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If82f974cc2d1e917464d4053563eaf4afc559150
Gerrit-Change-Number: 9494
Gerrit-PatchSet: 12
Gerrit-Owner: Nithya Janarthanan <njanartha...@cloudera.com>
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Nithya Janarthanan <njanartha...@cloudera.com>
Gerrit-Comment-Date: Sun, 11 Mar 2018 23:51:46 +0000
Gerrit-HasComments: Yes

Reply via email to