Closed Bug 884344 Opened 8 years ago Closed 8 years ago

Determine and document 3rd party package requirements for mozbase testing and how to hook them up

Categories

(Testing :: Mozbase, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: k0scist, Assigned: k0scist)

References

Details

Attachments

(1 file)

Currently, we don't really have anything detailing whether 3rd party
packages can or can not be used for testing or how they are
installed.  We should.
This was motivated by the addition of mock here:

https://bugzilla.mozilla.org/show_bug.cgi?id=881417

As discussed in the meeting, I think the easiest thing is just to forbid the use of anything not in the standard library. Anything else is just too complicated.
That's pretty crappy, mock is really nice for unit testing. :-(
Ok, after chatting with ted on irc it looks like we do have a mechanism for adding 3rd party packages into the tree for testing -- check them in, just as we do with mozbase. Then we install them into the m-c virtualenv. Apparently ted already did this with mock:

http://mxr.mozilla.org/mozilla-central/source/build/virtualenv/packages.txt#12

For the out of tree case, I assume we could just install mock as part of setup_development.py?

I still think we should try not to do this too often in general, but mock does indeed seem to be very useful. I think what we did there could be summarized and formulated into a policy, if that's important to peoples.
I guess we should at least add "tests_require": ["mock"]. I'm not 100% clear about what this actually does, assume it sets up modules if you run python setup.py test.
I mostly agree with :wlach . If a non-stdlib python package really provides a lot of value, then I think it is worth (conservatively) considering for inclusion.  But there is overhead.  We also don't have a strategy to deal with any of these, hence me trying to run the tests and getting:

(mozbase)│./test.py 
Traceback (most recent call last):
  File "./test.py", line 87, in <module>
    main()
  File "./test.py", line 75, in main
    unittestlist.extend(unittests(test))
  File "./test.py", line 33, in unittests
    module = imp.load_source(modname, path)
  File "/home/jhammel/mozbase/src/mozbase/mozinfo/tests/test.py", line 8, in <module>
    import mock
ImportError: No module named mock

As per comment 3, I think at the least we should add these to `setup_development.py`. 

I also agree with :wlach that we shouldn't do this whimfully and in the future we should consider each module.  I should have noted this in the review, but iirc time was pressing.

I would love to get `python setup.py test` working.  Indeed, `tests_require` will allow automagic (and temporary) test-dependency resolution for `python setup.py test`.  However (sadface), the tests have to be part of the module as I understand it: http://pythonhosted.org/distribute/setuptools.html#test-build-package-and-run-a-unittest-suite .  IMHO this is not only a bad idea (overhead, need to actually account for tests in package structure, package data, etc), it is not the case we have now.  I don't know a way around this, but if someone did....I'd love to straighten our story there.

Also, for the record, we currently (and completely silently) in a few cases use mozbase packages not required by the package in question for their tests.  Again, I don't see anything wrong with this, but it would be good to note.
So I'm going to advocate the following for purposes of this bug's resolution:

- add 'mock' to setup_development.py --extra even though i will constantly forget to run the damn thing; probably also document this on https://wiki.mozilla.org/Auto-tools/Projects/Mozbase#Installing_Mozbase ... oh wait i just did

- document this on https://wiki.mozilla.org/Auto-tools/Projects/Mozbase

- ticket the python setup.py test dream; it may not be possible, but at least good to have on record why we do the things we do

I'm happy to take this, or if someone else wants happy to hand over
Assignee: nobody → jhammel
Attachment #764842 - Flags: review?(ahalberstadt)
Thanks for cleaning up after me. I would like to propose, as a first cut, the litmus test for useful modules be "has been included in the Python standard library in a more recent Python release", which mock would pass.
We are trying to get mozbase tests running during 'make check'; how will we deal with external dependencies there?
Per comment 3, we can add them to the virtualenv in the objdir by checking them in like I have already done with mock.
Comment on attachment 764842 [details] [diff] [review]
fix setup_development.py

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

Thanks!

I wonder if there's some way we could use the "extras_require" feature (http://pythonhosted.org/distribute/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies), can't think of a good solution otoh.
Attachment #764842 - Flags: review?(ahalberstadt) → review+
one thing I don't understand is how our testing instance did fail on this;  is mock installed there by default?
(In reply to Jeff Hammel [:jhammel] from comment #12)
> one thing I don't understand is how our testing instance did fail on this; 
> is mock installed there by default?

Didn't you know? It can mock it's own import in case it isn't installed
(In reply to Andrew Halberstadt [:ahal] from comment #13)
> (In reply to Jeff Hammel [:jhammel] from comment #12)
> > one thing I don't understand is how our testing instance did fail on this; 
> > is mock installed there by default?
> 
> Didn't you know? It can mock it's own import in case it isn't installed

Ah, like my @require decorator:

@require("require")
def require(method, *deps):
    """decorate a function as requiring a pypi package"""
    # XXX note that this is done recursively; this is tricky
    import require
    return require.require
Apparently ted already checked mock into the tree:

> Per comment 3, we can add them to the virtualenv in the objdir by checking
> them in like I have already done with mock.

Hence, it works.
(In reply to Jonathan Griffin (:jgriffin) from comment #15)
> Apparently ted already checked mock into the tree:
> 
> > Per comment 3, we can add them to the virtualenv in the objdir by checking
> > them in like I have already done with mock.
> 
> Hence, it works.

Assuming you're refering to comment 12, I was asking about our travis instance which does not run from mozilla-central or the like.
(In reply to Jeff Hammel [:jhammel] from comment #6)
<snip/>
> - ticket the python setup.py test dream; it may not be possible, but at
> least good to have on record why we do the things we do
> 
> I'm happy to take this, or if someone else wants happy to hand over

https://bugzilla.mozilla.org/show_bug.cgi?id=885342
See Also: → 885342
(In reply to Andrew Halberstadt [:ahal] from comment #11)
> Comment on attachment 764842 [details] [diff] [review]
> fix setup_development.py
> 
> Review of attachment 764842 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks!
> 
> I wonder if there's some way we could use the "extras_require" feature
> (http://pythonhosted.org/distribute/setuptools.html#declaring-extras-
> optional-features-with-their-own-dependencies), can't think of a good
> solution otoh.

pushed: https://github.com/mozilla/mozbase/commit/28d1e282390f44d52f810c1eb3fca4ff3f91ab81

I don't see how "extras_require" solves anything here, but feel free to elaborate on bug 885342
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.