Page MenuHomeSoftware Heritage
Differential D3475

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

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

Details

Reviewers
anlambert
olasd
Group Reviewers
Reviewers
Maniphest Tasks
T2484: Move sharable fixtures out of conftest into a dedicated pytest plugin
Commits
rDSTO23318c21d101: setup: Do no expose the pytest-plugin any longer
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
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13520
Build 20690: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 20689: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
100,163 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.algos.test_snapshot::test_snapshot_large
swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f2cfba2ccf8> @given(branch_name=branch_names(), branch_target=branch_targets(only_objects=True))
4 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.fixer::swh.storage.fixer._fix_content
2 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.fixer::swh.storage.fixer._fix_origin
4 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.fixer::swh.storage.fixer._fix_origin_visit
4 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.fixer::swh.storage.fixer._fix_revision
View Full Test Results (1 Failed · 779 Passed · 17 Skipped)

Event Timeline

ardumont created this revision.Jul 9 2020, 10:22 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptJul 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.

swh-public-ci added a comment.Jul 9 2020, 10:30 AM

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
Harbormaster failed remote builds in B13509: Diff 12290!Jul 9 2020, 10:30 AM
ardumont added a comment.Jul 9 2020, 10:37 AM

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
douardda added a subscriber: douardda.Jul 9 2020, 10:43 AM
In D3475#85434, @ardumont wrote:

Adapt according to remarks:

  • Update docstring

what docstring?

ardumont added a comment.Jul 9 2020, 10:43 AM

what docstring?

oops, sorry "git commit message"

ardumont edited the summary of this revision. (Show Details)Jul 9 2020, 10:44 AM
swh-public-ci added a comment.Jul 9 2020, 10:49 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
Harbormaster failed remote builds in B13511: Diff 12292!Jul 9 2020, 10:49 AM
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
swh-public-ci added a comment.Jul 9 2020, 2:45 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
Harbormaster failed remote builds in B13520: Diff 12301!Jul 9 2020, 2:45 PM
ardumont edited the summary of this revision. (Show Details)Jul 9 2020, 3:52 PM
ardumont mentioned this in D3479: Require explicitely the dependencies on storage fixtures.Jul 9 2020, 4:28 PM
ardumont edited the summary of this revision. (Show Details)Jul 9 2020, 4:33 PM
ardumont mentioned this in D3481: Clean up the swh.scheduler / swh.storage pytest plugin imports.Jul 9 2020, 5:32 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
swh-public-ci added a comment.Jul 9 2020, 6:41 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.
Harbormaster completed remote builds in B13552: Diff 12331.Jul 9 2020, 6:41 PM
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
swh-public-ci added a comment.Jul 9 2020, 6:57 PM

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.
Harbormaster completed remote builds in B13554: Diff 12333.Jul 9 2020, 6:57 PM
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

swh-public-ci added a comment.Jul 10 2020, 8:26 AM

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.
Harbormaster completed remote builds in B13563: Diff 12340.Jul 10 2020, 8:26 AM
Closed by commit rDSTO23318c21d101: setup: Do no expose the pytest-plugin any longer (authored by ardumont). · Explain WhyJul 10 2020, 8:29 AM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDSTO23318c21d101: setup: Do no expose the pytest-plugin any longer.
ardumont mentioned this in D3492: setup: Do no expose the pytest-plugin, let modules require it.Jul 10 2020, 12:11 PM
ardumont mentioned this in rDSCH254e24a68780: Do no expose pytest-plugin through setuptools, let modules require it when….Jul 10 2020, 1:00 PM

Revision Contents
Changeset List

PathSize
2 lines
swh/
storage/
tests/
4 lines

Diff 12301

View Options

setup.py

Loading...
View Options

swh/storage/tests/conftest.py

Loading...
About · Developer information · Legal