Nithya Janarthanan 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 4:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/9494/4//COMMIT_MSG@7
PS4, Line 7: IMPALA-5886 & IMPALA-4812 Update run-tests.py script to handle
> Agreed. And also, please explain testing done.
It has been handled in the latest patch


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

http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@30
PS4, Line 30: from _pytest.runner import runtestprotocol
> Is this used?
Again...handled in the last patch


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@65
PS4, Line 65: class TestStatisticsPlugin:
> Please use new-style classes, and inherit from object.
Done.

I would prefer to keep the name as TestStatistics , so  any functions related 
to pytest data can be added here


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@65
PS4, Line 65: class TestStatisticsPlugin:
> I see what you are saying. But the other classes (Class Testexecutor) in ru
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@67
PS4, Line 67:   def __init__(self):
            :       self.tests_collected = []
            :       self.tests_executed = []
> 2 spaces, please.
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@71
PS4, Line 71: # items represents the list of collected test items
> Agreed.
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@72
PS4, Line 72:   def pytest_collection_modifyitems(self, items):
> For these special hooks I think it's important to note that the name is sig
Done


http://gerrit.cloudera.org:8080/#/c/9494/4/tests/run-tests.py@97
PS4, Line 97:       for test in plugin.tests_collected:
            :         print(test)
> Not sure this is necessary. It looks like you're printing tests twice: once
I have kept this to allow suppressing of pytest verbose messages and just see 
the list of collected tests by

run-tests.py shell --collect-only -p no:terminal



--
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: 4
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: Wed, 07 Mar 2018 23:20:34 +0000
Gerrit-HasComments: Yes

Reply via email to