Add mock to mozbase's setup

RESOLVED FIXED

Status

Testing
Mozbase
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ffledgling, Assigned: wlach)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
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.
Flags: needinfo?(jhammel)
(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.
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.
(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.
Created attachment 779816 [details] [diff] [review]
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.
Assignee: nobody → wlachance
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.
Attachment #779816 - Flags: review?(jhammel)

Comment 7

5 years ago
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
Flags: needinfo?(jhammel)

Comment 8

5 years ago
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
Attachment #779816 - Flags: review?(jhammel) → review+
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
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.