Nithya Janarthanan has posted comments on this change. ( )

Change subject: IMPALA-5886 & IMPALA-4812 Update to handle 
exit_code 5

Patch Set 4:

Commit Message:
PS4, Line 7: IMPALA-5886 & IMPALA-4812 Update script to handle
> Agreed. And also, please explain testing done.
It has been handled in the latest patch
File tests/
PS4, Line 30: from _pytest.runner import runtestprotocol
> Is this used?
Again...handled in the last patch
PS4, Line 65: class TestStatisticsPlugin:
> Please use new-style classes, and inherit from object.

I would prefer to keep the name as TestStatistics , so  any functions related 
to pytest data can be added here
PS4, Line 65: class TestStatisticsPlugin:
> I see what you are saying. But the other classes (Class Testexecutor) in ru
PS4, Line 67:   def __init__(self):
            :       self.tests_collected = []
            :       self.tests_executed = []
> 2 spaces, please.
PS4, Line 71: # items represents the list of collected test items
> Agreed.
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
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 shell --collect-only -p no:terminal

To view, visit
To unsubscribe, visit

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 <>
Gerrit-Reviewer: David Knupp <>
Gerrit-Reviewer: Fredy Wijaya <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-Reviewer: Nithya Janarthanan <>
Gerrit-Comment-Date: Wed, 07 Mar 2018 23:20:34 +0000
Gerrit-HasComments: Yes

Reply via email to