search

cdent / gabbi

Declarative HTTP Testing for Python and anything else
http://gabbi.readthedocs.org/
Other
148 stars 34 forks source link

Add tox to test requirements #233

Closed joshleeb closed 6 years ago

joshleeb commented 6 years ago

I came across this during the development of #232. However, I thought it wasn't relevant so I would add it to a separate PR.

cdent commented 6 years ago

A while back, someone made a pull request to remove tox from test-requirements.txt in 30649f6c977ab68b8fb546ea5d350949acf6227e

The rationale at the time, I think, was that unless you already had tox you wouldn't engage test-requirements.txt anyway.

I guess it depends on how things are ordered and what's doing the automation. What was your path to discovering the problem?

joshleeb commented 6 years ago

I first created a new virtualenv and installed gabbi with:

$ pip install -e . -r requirements.txt -r test-requirements.txt

Then, to run the tests, I noticed there was a tox.ini file that was setup to run tests, so I tried to run tox as a quick way to run the tests. And that's where I hit this snag.

I also found it useful to run tox before submitting a PR to know, with reasonable certainty, if the CI jobs will pass.

It isn't really a big deal but I imagine for most people who setup a new virtualenv to contribute to gabbi they would run into this issue.

cdent commented 6 years ago

It isn't really a big deal but I imagine for most people who setup a new virtualenv to contribute to gabbi they would run into this issue.

I think you're probably right that having it is useful. I'm cc'ing @FND and @jasonamyers to see if they have any memory of when it went away.

jasonamyers commented 6 years ago

It went away exactly like you mentioned. Often tox is installed at the user or system level no need to reinstall it during testing as you be reinstalling tox into each tox test environment tox. It would make sense to have tox in requirements or requirements-dev.txt

FND commented 6 years ago

Unfortunately, I don't have any memory. Nevertheless, as what amounts to a casual contributor at this point, I would prefer gabbi's dependency declarations to be comprehensive, without relying on global/system-level packages - i.e. I would expect pretty much what @joshleeb described. (I vaguely remember @cdent having to guide me through the setup process more than once - which neither of us particularly enjoyed.)

cdent commented 6 years ago

It's always the simplest things that are complex. 🤦‍♂️ It appears that requirements-dev.txt is relatively common, should we add it and put tox in there instead of test-requirements.txt?

joshleeb commented 6 years ago

That seems reasonable to me. Maybe name it dev-requirements.txt to keep consistency with test-requirements.txt?

That way it can be an option not to install tox (or not to install it again) and if a contributor decides to run pip with all the *requirements.txt files then they would be fine there too.

jasonamyers commented 6 years ago

It’s definitely more common to have requirements.txt and requirements-dev.txt. Often it's encouraged that we shouldn’t be installing test-requirements.txt and that it should be used by tox only. Since that’s the desired testing route.

joshleeb commented 6 years ago

I'd be happy to update the PR to split requirements into requirements.txt and requirements-dev.txt?

I'm thinking requirements.txt wouldn't change. Then test-requirements.txt would become requirements-dev.txt and include tox.

cdent commented 6 years ago

I'm thinking requirements.txt wouldn't change. Then test-requirements.txt would become requirements-dev.txt and include tox.

Sorry, slightly more complex than that:

I've got a bad cold, I'm not processing information well.

jasonamyers commented 6 years ago

So Tox needs a set of requirements, and in my past experience that does not include tox. So we'd have requirements.txt for running it, requirements-dev.txt to have all the things developers need, and test-requirements.txt for all the tox needs. I'd imagine requirements-dev.txt to -r test-requirements.txt and include tox.

jasonamyers commented 6 years ago

:+1: then good to remove it

On Tue, Dec 19, 2017 at 8:07 AM, Chris Dent notifications@github.com wrote:

@cdent commented on this pull request.

In requirements-dev.txt https://github.com/cdent/gabbi/pull/233#discussion_r157763023:

@@ -0,0 +1,3 @@ +-r test-requirements.txt

That is the question I was trying to get an answer for earlier; whether there is anything beyond tox that's required. So I guess I'll just try it. One moment... tox, git, and an editor.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cdent/gabbi/pull/233#discussion_r157763023, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKl-QUo6rMrXOso5HHigg2nhutw4MANks5tB8MvgaJpZM4REIks .

cdent commented 6 years ago

Thanks for sticking with this @joshleeb . I'll make some tweaks in the docs to mention its existence before I package up the release that includes the header changes you made (thanks very much for those too).