Page MenuHomeSoftware Heritage

requirements-test.txt: Drop no longer needed test dependency
ClosedPublic

Authored by ardumont on Nov 23 2020, 9:59 AM.

Details

Summary

It's already pulled from swh.core[db] which is a runtime dependency.

Related to T2746

Test Plan

tox

Diff Detail

Event Timeline

Build is green

Patch application report for D4554 (id=16159)

Could not rebase; Attempt merge onto 29db82c650...

Updating 29db82c..42ac003
Fast-forward
 requirements-test.txt          |  2 +-
 swh/vault/tests/test_server.py | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)
Changes applied before test
commit 42ac00365bdc9495de52a4af4b5d7196ec90b796
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Nov 23 09:56:27 2020 +0100

    requirements-test.txt: Use correct test dependency swh.core[db]
    
    Instead of pytest-postgresql.
    
    Related to T2746

commit 516e5956c9f5d47f979ecc17b98a529ea58542cf
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Nov 23 09:48:48 2020 +0100

    test_server: Fix exception structure
    
    swh.core (0.9.0) changed the remote exception serialization handling.
    
    So the current build [1] is broken as it needs adaptation to retrieve the
    exception information at its first level.
    
    This adapts accordingly the exception handling in those failing tests.
    
    [1] https://jenkins.softwareheritage.org/job/DVAU/job/tests/881/

See https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/54/ for more details.

douardda added a subscriber: douardda.

swh.core[db] is already a dep in requirements-swh.txt, why would is be needed here?

But more importantly, I disagree with the validity of the diff because pytest-posgtgresql is used directly in this package (in conftest.py), not only by the mean of the swh.core.tests.db package.

This revision now requires changes to proceed.Nov 23 2020, 10:40 AM

swh.core[db] is already a dep in requirements-swh.txt, why would is be needed here?

But more importantly, I disagree with the validity of the diff because pytest-posgtgresql is used directly in this package (in conftest.py), not only by the mean of the swh.core.tests.db package.

(I wrote a bit too fast, I missed the imported postgresql factory from swh.core.pytest_plugin however) the fixture from pytest-postgresql is also used as is. So the dependency is not (only) transitive, so IMHO the dependency should stay.

But more importantly, I disagree with the validity of the diff because pytest-posgtgresql is used directly in this package (in conftest.py),

no, it's not used directly.

(I wrote a bit too fast, I missed the imported postgresql factory from swh.core.pytest_plugin however)

heh, that ^ for the previous point, ok.

the fixture from pytest-postgresql is also used as is.

no, that fixture adds the resets those tables behavior which is not what's the original fixture does.

So the dependency is not (only) transitive, so IMHO the dependency should stay.

I disagree because see my previous point.

I'm trying hard to make sense of our spaghetti deps here... And i'm trying to
make it have some sense both here and in the debian package dimension...

swh.core[db] allows, in the python dimension to expose the
swh.core.db.pytest_plugin test fixture, thus why it's declared explicitely
here.

In the debian dimension, the build dependency that needs to be used for my last
sentence is python3-swh.core.db.pytestplugin (D4555 for an example on scheduler).

Note that for the requirement tests, i hesitated to add the swh.core[testing]
instead of swh.core[db] but i'm unsure that it makes more sense... [1]

[1] https://forge.softwareheritage.org/D4548#inline-31239

Also, debian package wise, if I don't drop that dependency here, it makes my previous work on the debian package from core moot.
As it will pull again the python3-pytest-postgresql (and thus postgresql).
This time, not from the core package but by the vault package...

And following the current logic, this ^ wrong behavior will be true for other packages i started fixing (scheduler, ...).

Also, debian package wise, if I don't drop that dependency here, it makes my previous work on the debian package from core moot.
As it will pull again the python3-pytest-postgresql (and thus postgresql).
This time, not from the core package but by the vault package...
And following the current logic, this ^ wrong behavior will be true for other packages i started fixing (scheduler, ...).

Ah no, great, it's not to a problem because it's a test dependency here, not a runtime one
As per D4555's test plan shows.


My previous comment still holds though.

requirements-test.txt: Drop no longer needed test dependency

It's already pulled from swh.core[db] which is a runtime dependency.

ardumont retitled this revision from requirements-test.txt: Use correct test dependency swh.core[db] to requirements-test.txt: Drop no longer needed test dependency.Nov 23 2020, 12:29 PM
ardumont edited the summary of this revision. (Show Details)

Build is green

Patch application report for D4554 (id=16178)

Rebasing onto 516e5956c9...

Current branch diff-target is up to date.
Changes applied before test
commit db8f2ab792ab2802a2d0e0ac13dce7f407a4f936
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Nov 23 09:56:27 2020 +0100

    requirements-test.txt: Drop no longer needed test dependency
    
    It's already pulled from swh.core[db] which is a runtime dependency.
    
    Related to T2746

See https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/55/ for more details.

Actually drop the dependency instead of just talking about it ¯\_(ツ)_/¯

Build is green

Patch application report for D4554 (id=16179)

Rebasing onto 516e5956c9...

Current branch diff-target is up to date.
Changes applied before test
commit 96fb83830a2c0e3f42a0c47c0e17c85e81ed752e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Nov 23 09:56:27 2020 +0100

    requirements-test.txt: Drop no longer needed test dependency
    
    It's already pulled from swh.core[db] which is a runtime dependency.
    
    Related to T2746

See https://jenkins.softwareheritage.org/job/DVAU/job/tests-on-diff/56/ for more details.

This revision is now accepted and ready to land.Nov 23 2020, 12:42 PM