896806 - Add mock to mozbase's setup

Status: RESOLVED FIXED

(Testing :: Mozbase, defect)

Description

[Anhad Jai Singh (:ffledgling)]

Mock is now used for testing in mozbase, but the setup_development script doesn't install mock as part of the dependencies for mozbase. This should be fixed.

Comment 1

[Andrew Halberstadt [:ahal]]

It will if you run 'python setup_development.py --extra'. I believe we'll want to wontfix this, but I'll let Jeff comment in case he had ideas of making things better.

Comment 2

[William Lachance (:wlach)]

(In reply to Andrew Halberstadt [:ahal] from comment #1)
> It will if you run 'python setup_development.py --extra'. I believe we'll
> want to wontfix this, but I'll let Jeff comment in case he had ideas of
> making things better.

Hmm, so you need to run what's already a developer-only commmand in a non-standard way just to make the unit tests work? That doesn't seem right at all to me.

Comment 3

[Andrew Halberstadt [:ahal]]

I think we moved things like that to the extras because they were taking too long to install (I don't think mock is actually a problem, but sphinx could be pretty annoying).

I think what should really happen is that we run the tests via 'python setup.py test' which then looks for 'tests_require' and bootstraps any needed modules before running them. I think we talked about this before, but it was just a matter of no one has time.

Comment 4

[William Lachance (:wlach)]

(In reply to Andrew Halberstadt [:ahal] from comment #3)

> I think what should really happen is that we run the tests via 'python
> setup.py test' which then looks for 'tests_require' and bootstraps any
> needed modules before running them. I think we talked about this before, but
> it was just a matter of no one has time.

Well, that would restrict you to running the tests via 'python setup.py test', which isn't always what you want.

Anyway, I don't think we should let waiting for a perfect solution to materialize get in the way of a concrete problem for new contributors right now. The modification required to install test dependencies by default is just a few lines, let's just do that.

Comment 5

[William Lachance (:wlach)]

Attachment: 0001-Bug-896806-Install-packages-required-for-testing-by-.patch

This patch installs packages required for testing (currently just mock) by default. I would have rather added the dependency to the mozcrash package under "tests_require" and then introspected on that when gathering dependencies, but it seems like tests_require doesn't make it into the package egg info anywhere.

I'm sure better approaches are possible, but this patch at least makes the unit tests runnable by default, which will hopefully result in less confusion for people.

Comment 6

[William Lachance (:wlach)]

Comment on attachment 779816 [details] [diff] [review]
0001-Bug-896806-Install-packages-required-for-testing-by-.patch

I'll set jhammel as reviewer, in case he has better ideas.

Comment 7

[Jeff Hammel]

iirc, it was Andrew who thought the install (at the time only of sphinx) took too long.  personally i would always install the extras by default and have a e.g. --no-extras flag for those who think it takes too long.  Other than that, all said here is an accurate and complete picture AIUI

Comment 8

[Jeff Hammel]

Comment on attachment 779816 [details] [diff] [review]
0001-Bug-896806-Install-packages-required-for-testing-by-.patch

Of course this begs --test-extras and future complexity but whatever.  Fine for now

Comment 9

[William Lachance (:wlach)]

Yeah, we could spend lots of cycles trying to find the optimal configuration here. Hopefully this is an acceptable balance for now. :)

https://github.com/mozilla/mozbase/commit/cc824927a4829e4020b04705396609bb87fe3240