Page MenuHomeSoftware Heritage
Differential D2082

Add pytest fixture to allow mocking requests with data file
ClosedPublic
Actions

Authored by ardumont on Oct 8 2019, 10:44 AM.

Details

Reviewers
douardda
olasd
Group Reviewers
Reviewers
Commits
rDCORE3f7ab6eb3bc5: Add pytest fixture to allow mocking requests with data file
rDCORE61c831466f58: tox.ini: Use tests installed files instead of working directory
Summary

Roughly, the idea is that http queries are transformed into file queries
within http context. If found, the data file is served or not (404). This
translates into query response so that the code can actually complete as usual.

For sample use cases, see for examples:

Note:
The fixture's name is not that good, if you have any better idea, now is the time ;)

Test Plan

tox

Diff Detail

Repository
rDCORE Foundations and core functionalities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Oct 8 2019, 10:44 AM
swh-public-ci added a comment.Oct 8 2019, 10:46 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCORE/job/tox/297/
See console output for more information: https://jenkins.softwareheritage.org/job/DCORE/job/tox/297/console

Harbormaster failed remote builds in B8160: Diff 6991!Oct 8 2019, 10:46 AM
ardumont added a comment.Oct 8 2019, 10:55 AM

Build has FAILED

mmmph, possibly some python3.5 and pathlib issue or some such... investigating

ardumont updated this revision to Diff 6992.Oct 8 2019, 11:06 AM

Try and fix for python3.5

ardumont mentioned this in D2074: gnu.tests: Check gnu lister.Oct 8 2019, 11:08 AM
swh-public-ci added a comment.Oct 8 2019, 11:09 AM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/298/ for more details.

Harbormaster completed remote builds in B8161: Diff 6992.Oct 8 2019, 11:09 AM
ardumont edited the summary of this revision. (Show Details)Oct 8 2019, 11:51 AM
ardumont mentioned this in D2076: gnu: Separate listing and parsing logic and add integration test.Oct 8 2019, 1:10 PM
douardda requested changes to this revision.Oct 8 2019, 1:43 PM
douardda added a subscriber: douardda.
douardda added inline comments.
tox.ini
7–8

you can get rid of this I guess

This revision now requires changes to proceed.Oct 8 2019, 1:43 PM
olasd requested changes to this revision.Oct 8 2019, 1:57 PM
olasd added a subscriber: olasd.

While I understand what the fixture does, its documentation is lacking (you're only really documenting the inner callback, that users won't call into, not the fixture itself). I guess most of the documentation of the callback should move to the fixture itself?

For the fixture name, maybe something like requests_mock_from_datadir(_factory) / requests_mock_from_datadir_with_visits would be clearer (even though it's a mouthful)?

Finally, you should make sure that the rationale for adding a new toplevel module for the pytest plugin is really clear (the paste or the reference to pytest/2042 really aren't).

From my tests and reading documentation further (pytest, tox), the issue really is a limitation of the way we're using the tox + pytest combination: we're running the tests on the checked out version of the module, instead of the version that's installed by tox (which is what pytest expects). If we adapt tox.ini to run tests on the installed version of the module (by running pytest [...] {envsitepackagesdir}/swh/core/..., then we can ship the pytest plugin wherever we want without triggering the dreaded ImportMismatchError.

This is also how I have caught that the data files you're adding are not shipped in the sdist, and need to be added to MANIFEST.in.

olasd added a comment.Oct 8 2019, 2:04 PM
In D2082#48297, @olasd wrote:

Finally, you should make sure that the rationale for adding a new toplevel module for the pytest plugin is really clear (the paste or the reference to pytest/2042 really aren't).

From my tests and reading documentation further (pytest, tox), the issue really is a limitation of the way we're using the tox + pytest combination: we're running the tests on the checked out version of the module, instead of the version that's installed by tox (which is what pytest expects). If we adapt tox.ini to run tests on the installed version of the module (by running pytest [...] {envsitepackagesdir}/swh/core/..., then we can ship the pytest plugin wherever we want without triggering the dreaded ImportMismatchError.

(btw, I'm not saying that this diff should change that; just that we should think about cleaning up our tox config at some point)

ardumont added inline comments.Oct 8 2019, 2:27 PM
tox.ini
7–8

yup

ardumont added a comment.Oct 8 2019, 4:35 PM

For the fixture name, maybe something like requests_mock_from_datadir(_factory) / requests_mock_from_datadir_with_visits would be clearer (even though it's a mouthful)?

I'm kinda sold on requests_mock_datadir and requests_mock_datadir_visits, thx.

ardumont updated this revision to Diff 7024.Oct 8 2019, 4:53 PM

Rename local_get into requests_mock_datadir

And dependent factory or other fixtures with the same pattern name

swh-public-ci added a comment.Oct 8 2019, 4:57 PM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/299/ for more details.

Harbormaster completed remote builds in B8194: Diff 7024.Oct 8 2019, 4:57 PM
ardumont updated this revision to Diff 7030.Oct 8 2019, 5:21 PM

Try to improve the fixture factory's docstring

swh-public-ci added a comment.Oct 8 2019, 5:24 PM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/300/ for more details.

Harbormaster completed remote builds in B8200: Diff 7030.Oct 8 2019, 5:24 PM
ardumont updated this revision to Diff 7031.Oct 8 2019, 5:51 PM

Add missing test artifacts in MANIFEST.in

swh-public-ci added a comment.Oct 8 2019, 5:54 PM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/301/ for more details.

Harbormaster completed remote builds in B8201: Diff 7031.Oct 8 2019, 5:54 PM
olasd accepted this revision.Oct 8 2019, 7:54 PM
douardda added a comment.Oct 9 2019, 9:59 AM
In D2082#48297, @olasd wrote:

From my tests and reading documentation further (pytest, tox), the issue really is a limitation of the way we're using the tox + pytest combination: we're running the tests on the checked out version of the module, instead of the version that's installed by tox (which is what pytest expects). If we adapt tox.ini to run tests on the installed version of the module (by running pytest [...] {envsitepackagesdir}/swh/core/..., then we can ship the pytest plugin wherever we want without triggering the dreaded ImportMismatchError.

well this is the default behavior of tox: execute tests from local working directory against installed python package. However if this default makes sense for project which tests are not part of the package itself, we may indeed consider executing tests that are installed with the package. It would probably be a sensible choice.

I am inclined to give this a try before landing this diff. Thanks for pointing this out.

douardda added a comment.EditedOct 9 2019, 10:13 AM
In D2082#48580, @douardda wrote:
In D2082#48297, @olasd wrote:

From my tests and reading documentation further (pytest, tox), the issue really is a limitation of the way we're using the tox + pytest combination: we're running the tests on the checked out version of the module, instead of the version that's installed by tox (which is what pytest expects). If we adapt tox.ini to run tests on the installed version of the module (by running pytest [...] {envsitepackagesdir}/swh/core/..., then we can ship the pytest plugin wherever we want without triggering the dreaded ImportMismatchError.

well this is the default behavior of tox: execute tests from local working directory against installed python package. However if this default makes sense for project which tests are not part of the package itself, we may indeed consider executing tests that are installed with the package. It would probably be a sensible choice.

I am inclined to give this a try before landing this diff. Thanks for pointing this out.

Gave a quick test and it works fine (with the pytest plugin moved back within swh.core package).
The only drawback I've encountered is this prevent from specifying the test file to run via tox from cmd line (not easily at least), eg.

tox -e py3-core -- swh/core/tests/test_cli.py

won't work any more.

We may live with this limitation, since the test selection mechanism of pytest (using the -k option for example) still works:

tox -e py3-core  -- -k test_cli
ardumont updated this revision to Diff 7032.Oct 9 2019, 10:33 AM
  • tox.ini: Use tests installed files instead of working directory
  • Move pytest plugin within swh.core
ardumont edited the summary of this revision. (Show Details)Oct 9 2019, 10:33 AM
swh-public-ci added a comment.Oct 9 2019, 10:36 AM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/302/ for more details.

Harbormaster completed remote builds in B8202: Diff 7032.Oct 9 2019, 10:36 AM
ardumont updated this revision to Diff 7033.Oct 9 2019, 10:39 AM

Rephrase docstring

swh-public-ci added a comment.Oct 9 2019, 10:42 AM

Build is green
See https://jenkins.softwareheritage.org/job/DCORE/job/tox/303/ for more details.

Harbormaster completed remote builds in B8203: Diff 7033.Oct 9 2019, 10:42 AM
This revision was not accepted when it landed; it landed in state Needs Review.Oct 9 2019, 10:44 AM
Closed by commit rDCORE61c831466f58: tox.ini: Use tests installed files instead of working directory (authored by ardumont). · Explain Why
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDCORE61c831466f58: tox.ini: Use tests installed files instead of working directory.
ardumont added a commit: rDCORE3f7ab6eb3bc5: Add pytest fixture to allow mocking requests with data file.
ardumont mentioned this in D2097: pytest_plugin: Allow support for query parameters in get_response_cb.Oct 9 2019, 1:18 PM
ardumont mentioned this in rDLDG1ef4277fde72: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 2:44 PM
ardumont mentioned this in rDLDSVNdc19a5a8744d: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:04 PM
ardumont mentioned this in rDLDDEB110decc6f78f: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:08 PM
ardumont mentioned this in rDLDPYcdf7c94d63d5: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:10 PM
ardumont mentioned this in rDLDNPMbcb5d8432e8e: tox.ini: Fix py3 environment to use packaged tests.
ardumont mentioned this in rDLDDIR487c93fdcaa7: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:14 PM
ardumont mentioned this in rDLDTARd99278e581a0: tox.ini: Fix py3 environment to use packaged tests.
ardumont mentioned this in rDMODc103a6f9d3e2: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:17 PM
ardumont mentioned this in rDVAU52a0f1bb9d69: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:26 PM
ardumont mentioned this in rDOBJS7417c0888210: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:29 PM
ardumont mentioned this in rDJNLcd68b6769312: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:32 PM
ardumont mentioned this in rDSEAee0fd584cf82: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:42 PM
ardumont mentioned this in D2100: swh-graph: fixing ci: Fix tox.ini's py3 environment to use packaged tests.Oct 9 2019, 3:46 PM
ardumont mentioned this in D2101: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 3:55 PM
ardumont mentioned this in D2102: tox.ini: Fix py3 environment to use packaged tests.
ardumont mentioned this in rDLDHG75e670147c34: tox.ini: Fix py3 environment to use packaged tests.Oct 9 2019, 4:03 PM
ardumont mentioned this in rDLS4f30cb50e36c: tox.ini: Use tests installed files instead of working directory.Oct 9 2019, 5:39 PM
ardumont mentioned this in rDCIDXdd314d6a2cba: tox.ini: Fix py3 environment to use packaged tests.Oct 10 2019, 10:50 AM
ardumont mentioned this in rDSTO654a37e6bf5c: tox.ini: Fix py3 environment to use packaged tests.Oct 10 2019, 11:36 AM
ardumont mentioned this in rDSCH8eafc70a59bd: tox.ini: Use tests installed files instead of working directory.Oct 10 2019, 12:06 PM
ardumont mentioned this in D2115: tests: Explicit registering test tasks step for the swh_app.Oct 10 2019, 3:28 PM
ardumont mentioned this in rDSCHe770eb30f085: tests: Explicit registering test tasks step for the swh_app.Oct 10 2019, 3:36 PM
ardumont mentioned this in rDSTOC654a37e6bf5c: tox.ini: Fix py3 environment to use packaged tests.Oct 30 2019, 5:22 PM

Revision Contents
Changeset List

PathSize
8 lines

Diff 7034

View Options

tox.ini

Loading...
About · Developer information · Legal