709881 - test installation of talos via setup.py

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Jeff Hammel]

Currently, there is no tests for installation of talos via its
setup.py: http://hg.mozilla.org/build/talos/file/705a12151bf2/setup.py

In order to test this, you can do the following:

- make a virtualenv:
  https://developer.mozilla.org/en/Python/Virtualenv

- install talos into the virtualenv with setup.py; ensure that the
  exit code is zero

- try to import various talos pieces from the virtualenv's import
  paths; ensure these work

This will require net access, but I think that is fair.  This also
won't cover all cases, but it is a good thing to have and possibly
somethin good to build on.

Comment 1

[Jeff Hammel]

Attachment: test installation

It is really more of an integration test than a unittest.  Not sure if we'd rather use doctest here?  But it does seem to work

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 581125 [details] [diff] [review]
test installation

Review of attachment 581125 [details] [diff] [review]:
-----------------------------------------------------------------

Overall, this is great, thanks for getting started on some talos tests!

::: tests/test_install.py
@@ +42,5 @@
> +            # ensure we can import Talos
> +            self.assertCall([python, '-c', 'import talos'])
> +            self.assertCall([python, '-c', 'from talos import run_tests'])
> +
> +        finally:

no catch here?  why the try?  what are we expecting to fail and not catch?

@@ +92,5 @@
> +                scripts = os.path.join(path, subdir)
> +                if os.path.exists(scripts):
> +                    python = os.path.join(scripts, executable)
> +                    self.assertTrue(os.path.exists(python))
> +        finally:

we have the try/finally here, shouldn't we put a catch and print a failure out?  Otherwise we should document what we expect to show up in the try/finally blocks.

Comment 3

[Jeff Hammel]

the try-finally blocks are for cleanup.  We don't want lingering temporary files or directories around if something does go wrong.  In the former case, I accidentally left in debugging code here:

+        finally:
+            # cleanup
+            pass
+            print tempdir
+            #shutil.rmtree(tempdir)

Should just be:

finally:
    # cleanup
    shutil.rmtree(tempdir)

In general, anything could go wrong and I'm not sure what to do in the case it does.  I'm of the opinion that, since this is test code, if a call fails catastrophically ...then you let it.  Things that could go wrong that I can think of:

* syntax or other errors in setup.py [a legitimate failure]
* someone replaces setup.py with a directory called setup.py [wtf?]
* inappropriate permissions on setup.py [wtf?]
* no network connection [should probably err more gracefully, but you should get the urllib2 traceback]
* too many file handlers or processes open [system failure]
* someone wipes the temporary files during the test [wtf?]
* xcode not installed on Mac [see https://github.com/pypa/virtualenv/issues/168]
* a few other edge cases mostly involving a bad system environment

Things that could go wrong that I can't think of:
* ???

Outside of the legitimate failure of a setup.py syntax error, this is mostly "you're doing it wrong" edge cases.  I'm open to suggestions on how to fix these, but IMHO a strict check to ensure every possibility is a bit much for the test.  If we want to move in that direction, we should probably start working on a (shared) test harness.

I will update the patch to remove debugging code.  Is that sufficient to land?  Or do we want to deal with any of these edge cases?

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

I am fine removing the debugging code and landing.

Comment 5

[Jeff Hammel]

pushed: http://hg.mozilla.org/build/talos/rev/ec3d1c08542e