Test reliability fixes

[jmbowman]

Some fixes I needed just to get the existing needle tests to pass reliably when run locally (extracted from #63 ):

• Explicitly call send_remote_shutdown_command() for the PhantomJS service and clean up after it before calling driver.quit(); otherwise it sometimes gets called twice, throws an exception the second time, and leaves an orphaned process sitting around consuming CPU cycles. This is a workaround for this upstream selenium issue, which apparently isn't fixed in 3.3.1 as claimed.
• Print the output of the wrapped test runs from PluginTester subclasses. The PhantomJS shutdown failures mentioned above were sometimes triggering exceptions in the wrapped tearDownClass, so self.nose.success was False but there was no indication of why. This output is only shown in the console when the test actually fails, so it doesn't clutter up successful test runs. Some of the tests in my previous PR were still failing like this in Travis with no useful feedback, so hopefully this will help debug any remaining issues.
• Fixed the setup of the PluginTester subclasses so they pass when run in isolation (they had been relying on previous tests in the suite to create the baseline directory for them).

For reference, the exact traceback I was getting consistently for NeedleCleanupOnSuccessTest under Python 2.7:

.E
======================================================================
ERROR: test suite for <class 'red_box.RedBoxTestCase'>
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jeremybowman/Source/needle/.tox/py27/lib/python2.7/site-packages/nose/suite.py", line 228, in run
    self.tearDown()
  File "/Users/jeremybowman/Source/needle/.tox/py27/lib/python2.7/site-packages/nose/suite.py", line 351, in tearDown
    self.teardownContext(ancestor)
  File "/Users/jeremybowman/Source/needle/.tox/py27/lib/python2.7/site-packages/nose/suite.py", line 367, in teardownContext
    try_run(context, names)
  File "/Users/jeremybowman/Source/needle/.tox/py27/lib/python2.7/site-packages/nose/util.py", line 471, in try_run
    return func()
  File "/Users/jeremybowman/Source/needle/needle/cases.py", line 81, in tearDownClass
    cls.driver.quit()
  File "/Users/jeremybowman/Source/needle/.tox/py27/lib/python2.7/site-packages/selenium/webdriver/phantomjs/webdriver.py", line 76, in quit
    self.service.stop()
  File "/Users/jeremybowman/Source/needle/.tox/py27/lib/python2.7/site-packages/selenium/webdriver/common/service.py", line 149, in stop
    self.send_remote_shutdown_command()
  File "/Users/jeremybowman/Source/needle/.tox/py27/lib/python2.7/site-packages/selenium/webdriver/phantomjs/service.py", line 67, in send_remote_shutdown_command
    os.close(self._cookie_temp_file_handle)
OSError: [Errno 9] Bad file descriptor

----------------------------------------------------------------------
Ran 1 test in 1.380s

[jmbowman]

Well, it looks like this is a problem with selenium 3.3.1 in general (and maybe some earlier versions as well), and my fix isn't enough to reliably fix it (even though it works locally). I've added a comment to an upstream ticket which was just closed because they thought they fixed it, looking for a workaround in the meantime. It's always that same test (NeedleCleanupOnSuccessTest ) that runs afoul of it for me locally on Python 2.7, even on a fresh checkout of master with no changes applied; the test itself always passes, but the teardown fails when it tries to shut down PhantomJS twice for some reason.

[jmbowman]

Ok, this seems to be working better; sorry for the false start (both approaches fixed the problem for me in local testing). Let me know if you have any concerns with the current approach, or want me to squash the commits before merging.

[jphalip]

That's great. Can the print statements be removed now that you've fixed the issue? Also, it'd be great if you could squash the commits. Thanks!

[jmbowman]

I switched the print statements to debug logging; without this, there's no useful information available to help debug potential regressions in the future. Let me know if you want to go ahead and remove them completely instead.

[jphalip]

Ok I see. In this case, would it be ok to move the statement to setUp() or tearDown() instead of repeating it in each test method, and also add a comment to explain what it's exactly needed for?

[jmbowman]

How about this? I introduced a shared superclass for all the plugin tests which handles the logging, specifying the default test suite, and cleaning up the baseline image.

[jphalip]

Perfect, thanks for your thorough work on this!