Page MenuHomeSoftware Heritage

setup: Do no expose the pytest-plugin any longer
ClosedPublic

Authored by ardumont on Jul 9 2020, 10:22 AM.

Details

Summary

Defining the pytest-plugin though the pytest-plugin [1] makes it loaded by default.
This creates loading issues on modules depending on storage but not on
the pytest plugin storage exposes. It was explained in the doc and I did not realize [2]

Instead we'll explicitely define to modules depending on the pytest plugins in
their root conftest [3]:

pytest_plugins = [ "swh.storage.pytest_plugin" ]

The following fix remains with the current other modules [4] by:

  • adding their deps as swh.storage[testing] if not already.
  • drop the eventual duplication introduced in tox.ini for the now apparent wrong reasons ;)
  • remove the workaround to avoid loading stuff from storage where not needed (swh-web for one)

[1] https://docs.pytest.org/en/stable/writing_plugins.html#setuptools-entry-points

[2] https://docs.pytest.org/en/stable/writing_plugins.html#plugin-discovery-order-at-tool-startup

[3] https://docs.pytest.org/en/stable/writing_plugins.html#requiring-loading-plugins-in-a-test-module-or-conftest-file

[4] as an example of adaptations D3479 (mercurial loader)

Related to T2484

Test Plan

tox

Diff Detail

Repository
rDSTO Storage manager
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.Jul 9 2020, 10:22 AM
olasd added a subscriber: olasd.Jul 9 2020, 10:28 AM

The documentation should read pytest_plugins = ["swh.storage.pytest_plugin"]. You also need to add that line to your own conftest.py if you're depending on that fixture.

Build has FAILED

Patch application report for D3475 (id=12290)

Rebasing onto c21d0e3820...

Current branch diff-target is up to date.
Changes applied before test
commit ae0595f6f3ad5ba80da4a3ba5131ae86f0824c86
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jul 9 10:11:01 2020 +0200

    setup: Do no expose the pytest-plugin any longer
    
    This creates loading issues on modules depending on storage but not on the
    pytest plugin. Somehow the loading is eager and not lazy.
    
    Instead we'll explicitely define to modules depending on the pytest plugins in
    their root conftest:
pytest_plugin = [ "swh.storage.pytest_plugin" ]
```

[1]
https://docs.pytest.org/en/stable/writing_plugins.html#setuptools-entry-points

Related to T2484
Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/425/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/425/console

You also need to add that line to your own conftest.py if you're depending on that fixture.

I did not since we import * from the conftest.
But yeah, i guess that'd make sense we eat our own cake ;)

I'll adapt my typos in description and commit message.
Thanks ;)

ardumont edited the summary of this revision. (Show Details)Jul 9 2020, 10:37 AM
ardumont updated this revision to Diff 12292.Jul 9 2020, 10:41 AM

Adapt according to remarks:

  • Update docstring
  • use the pytest_fixtures definition ourselves

Adapt according to remarks:

  • Update docstring

what docstring?

what docstring?

oops, sorry "git commit message"

ardumont edited the summary of this revision. (Show Details)Jul 9 2020, 10:44 AM

Build has FAILED

Patch application report for D3475 (id=12292)

Rebasing onto c21d0e3820...

Current branch diff-target is up to date.
Changes applied before test
commit d94b05009b2f9887d5f005aa4e4fb13b9034ffb6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jul 9 10:11:01 2020 +0200

    setup: Do no expose the pytest-plugin any longer
    
    This creates loading issues on modules depending on storage but not on
    the pytest plugin storage exposes. Somehow the loading is eager and not lazy.
    
    Instead we'll explicitely define to modules depending on the pytest plugins in
    their root conftest:
pytest_plugins = [ "swh.storage.pytest_plugin" ]
```

[1]
https://docs.pytest.org/en/stable/writing_plugins.html#setuptools-entry-points

Related to T2484
Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/427/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/427/console
ardumont updated this revision to Diff 12301.Jul 9 2020, 2:38 PM

Rebase on latest master

anlambert accepted this revision.Jul 9 2020, 2:41 PM
anlambert added a subscriber: anlambert.

Looks good to me.

This revision is now accepted and ready to land.Jul 9 2020, 2:41 PM

Build has FAILED

Patch application report for D3475 (id=12301)

Rebasing onto c3803ef8f7...

Current branch diff-target is up to date.
Changes applied before test
commit 529259ffce0b4c0173025631ec11c53bd4ad8919
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jul 9 10:11:01 2020 +0200

    setup: Do no expose the pytest-plugin any longer
    
    This creates loading issues on modules depending on storage but not on
    the pytest plugin storage exposes. Somehow the loading is eager and not lazy.
    
    Instead we'll explicitely define to modules depending on the pytest plugins in
    their root conftest:
pytest_plugins = [ "swh.storage.pytest_plugin" ]
```

[1]
https://docs.pytest.org/en/stable/writing_plugins.html#setuptools-entry-points

Related to T2484
Link to build: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/434/
See console output for more information: https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/434/console
ardumont edited the summary of this revision. (Show Details)Jul 9 2020, 3:52 PM
ardumont edited the summary of this revision. (Show Details)Jul 9 2020, 4:33 PM
ardumont updated this revision to Diff 12331.Jul 9 2020, 6:34 PM

Rebase on latest master

ardumont edited the test plan for this revision. (Show Details)Jul 9 2020, 6:37 PM

Build is green

Patch application report for D3475 (id=12331)

Rebasing onto de38cd1126...

Current branch diff-target is up to date.
Changes applied before test
commit f1bb519de763b17cbf6bb07a2c3ba608fce0c759
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jul 9 10:11:01 2020 +0200

    setup: Do no expose the pytest-plugin any longer
    
    This creates loading issues on modules depending on storage but not on
    the pytest plugin storage exposes. Somehow the loading is eager and not lazy.
    
    Instead we'll explicitely define to modules depending on the pytest plugins in
    their root conftest:
pytest_plugins = [ "swh.storage.pytest_plugin" ]
```

[1]
https://docs.pytest.org/en/stable/writing_plugins.html#setuptools-entry-points

Related to T2484
See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/439/ for more details.
ardumont updated this revision to Diff 12333.Jul 9 2020, 6:51 PM
  • Move the pytest_plugins variable definition to the root conftest.py
  • Rework commit message to align with the diff description

Build is green

Patch application report for D3475 (id=12333)

Rebasing onto de38cd1126...

Current branch diff-target is up to date.
Changes applied before test
commit 9465ad8bf400ea58d5290c2ca8d07eaa3af32670
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jul 9 10:11:01 2020 +0200

    setup: Do no expose the pytest-plugin any longer
    
    Defining the pytest-plugin though the pytest-plugin [1] makes it loaded by default.
    This creates loading issues on modules depending on storage but not on
    the pytest plugin storage exposes. It was explained in the doc and I did not realize [2]
    
    Instead we'll explicitely define to modules depending on the pytest plugins in
    their root conftest [3]:
pytest_plugins = [ "swh.storage.pytest_plugin" ]
```

[1] https://docs.pytest.org/en/stable/writing_plugins.html#setuptools-entry-points

[2] https://docs.pytest.org/en/stable/writing_plugins.html#plugin-discovery-order-at-tool-startup

[3] https://docs.pytest.org/en/stable/writing_plugins.html#requiring-loading-plugins-in-a-test-module-or-conftest-file

Related to T2484
See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/440/ for more details.
olasd accepted this revision.Jul 9 2020, 7:29 PM
ardumont updated this revision to Diff 12340.Jul 10 2020, 8:19 AM

Rebase on latest master

Build is green

Patch application report for D3475 (id=12340)

Rebasing onto 124e76d294...

Current branch diff-target is up to date.
Changes applied before test
commit 23318c21d1017b4d18a18e176bca08c180908571
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jul 9 10:11:01 2020 +0200

    setup: Do no expose the pytest-plugin any longer
    
    Defining the pytest-plugin though the pytest-plugin [1] makes it loaded by default.
    This creates loading issues on modules depending on storage but not on
    the pytest plugin storage exposes. It was explained in the doc and I did not realize [2]
    
    Instead we'll explicitely define to modules depending on the pytest plugins in
    their root conftest [3]:
pytest_plugins = [ "swh.storage.pytest_plugin" ]
```

[1] https://docs.pytest.org/en/stable/writing_plugins.html#setuptools-entry-points

[2] https://docs.pytest.org/en/stable/writing_plugins.html#plugin-discovery-order-at-tool-startup

[3] https://docs.pytest.org/en/stable/writing_plugins.html#requiring-loading-plugins-in-a-test-module-or-conftest-file

Related to T2484
See https://jenkins.softwareheritage.org/job/DSTO/job/tests-on-diff/441/ for more details.
This revision was automatically updated to reflect the committed changes.