-----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 <[email protected]> >> 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----- _______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
