Open Stack

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 06/18/2013 01:19 PM, Roman Bogorodskiy wrote:
> Robert Collins wrote:
> 
>> On 19 June 2013 03:56, Clark Boylan <clark.boylan at gmail.com>
>> wrote:
>> 
>>> I finally got around to looking into this today. I ran `tox
>>> -epy27` locally a couple times and noticed that test numbers
>>> did vary (but not as wildly as reported earlier). My next step
>>> was to source the tox py27 virtualenv then to `testr list-tests
>>> | wc -l` to get a rough number for the total number of tests
>>> (if tests are dynamically created at run time this number will
>>> be low). This gave me 5986 well above the ~5500-5600 that were
>>> actually running. At this point I wanted to remove parallel
>>> testing from the equation to see if all tests run properly in
>>> serial. `testr run` ran 2368 tests so removing parallel testing
>>> did not fix the problem.
>>> 
>>> Running the tests in this way did produce some interesting
>>> output in the subunit log [1]. Notice that the NEC agent is
>>> returning 0. This kills the test runner prematurely and tells
>>> testr that tests passed. This sys.exit(0) shouldn't be in the
>>> test path like this. I haven't looked at what fixing this will
>>> entail but it is probably possible to catch the SystemExit
>>> exception in the test and ignore it. I have a hunch the proper
>>> fix will involve making the NEC agent more properly unit
>>> testable. I will defer to those with more knowledge of
>>> OpenStack Networking for the actual fix.
>> 
>> So, I think try: except SystemExit: ...
>> 
>> in the test is a good first approximation - this should be the
>> first step.
>> 
>> Clark and I chatted on IRC... it's pretty bad form to catch
>> SystemExit in library frameworks - it gets in the way of anyone
>> *wanting* to stop a process. It's possible we can do something
>> reasonably tasteful where we catch it, emit a test failure, and
>> then rethrow it; please feel free to file a bug (on testtools),
>> but its definitely not as trivial as 'start catching it like any
>> other exception'.
>> 
>> -Rob
> 
> Do you mean we need to catch SystemExit in the actual unit test?
> 
> The other option I guess is to either move sys.exit() to __main__
> block or just mock it.
> 
> I have submitted a change like that for the nec_agent test:
> 
> https://review.openstack.org/#/c/33479/
> 
> What do you think about it?

Yes! I think this is exactly how that should be handled.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iEYEARECAAYFAlHAmvQACgkQ2Jv7/VK1RgFu4wCfXAlNPncZ50LMcRMeh2ItNH52
z1IAoI6YiP/zSwXGMP2T8EhJzzwROQm1
=ylIE
-----END PGP SIGNATURE-----