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 13509
Build 20670: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 20669: arc lint + arc unit

Unit TestsFailed
View All

TimeTest
100,833 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 0x7f53d4dac860> @given(branch_name=branch_names(), branch_target=branch_targets(only_objects=True))
611 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_object_find_by_sha1_git
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f53c3e58b70> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f53c3e6f4a8>
561 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_origin_visit_get_by
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f54982d1ef0> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f54b8214048>
526 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_origin_visit_status_add
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f55181b70f0> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f54d80a0ac8>
578 msJenkins > .tox.py3.lib.python3.7.site-packages.swh.storage.tests.test_cassandra.TestCassandraStorage::test_origin_visit_status_get_latest
self = <swh.storage.tests.test_cassandra.TestCassandraStorage object at 0x7f54984ca588> swh_storage = <swh.storage.validate.ValidatingProxyStorage object at 0x7f54981a37f0>
View Full Test Results (16 Failed · 764 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

Diff 12290

View Options

setup.py

Loading...
About · Developer information · Legal