⚙ D5467 tox: Check swh-identify can run even if Dulwich isn't installed

vlorentz created this revision.Apr 8 2021, 7:18 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptApr 8 2021, 7:18 PM

Build is green

Patch application report for D5467 (id=19536)

Could not rebase; Attempt merge onto eeedac7af7...

Updating eeedac7..369f5a3
Fast-forward
 swh/model/cli.py | 15 +++++++++++++--
 tox.ini          | 21 ++++++++++++++++++++-
 2 files changed, 33 insertions(+), 3 deletions(-)

Changes applied before test

commit 369f5a37d695e473749c5e7fa4c90186e5ded141
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Apr 8 19:18:10 2021 +0200

    tox: Check swh-identify can run even if Dulwich isn't installed

commit 8d4eda816e893805e78c24cdc9e5ad979222a7c3
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Apr 8 18:55:01 2021 +0200

    swh-identify: Hide tracebacks if Click or Dulwich is not installed
    
    And show nice human-readable errors instead

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

Harbormaster completed remote builds in B20573: Diff 19536.Apr 8 2021, 7:19 PM

vlorentz requested review of this revision.Apr 8 2021, 7:19 PM

rebase

Build is green

Patch application report for D5467 (id=19540)

Could not rebase; Attempt merge onto eeedac7af7...

Updating eeedac7..f2e3238
Fast-forward
 swh/model/cli.py | 18 ++++++++++++++++--
 tox.ini          | 21 ++++++++++++++++++++-
 2 files changed, 36 insertions(+), 3 deletions(-)

Changes applied before test

commit f2e323830841c2aa5f1cdada1781e6f483830109
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Apr 8 19:18:10 2021 +0200

    tox: Check swh-identify can run even if Dulwich isn't installed

commit 3e4bf9cb108d30fe2a727acdef69c61fa6b1f1d0
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Apr 8 18:55:01 2021 +0200

    swh-identify: Hide tracebacks if Click or Dulwich is not installed
    
    And show nice human-readable errors instead

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

Harbormaster completed remote builds in B20577: Diff 19540.Apr 8 2021, 7:25 PM

ardumont added a subscriber: ardumont.Apr 9 2021, 10:12 AM

ardumont added inline comments.

tox.ini
32	?

ardumont added inline comments.Apr 9 2021, 10:16 AM

tox.ini

32

I usually use -k for inclusion with the name of the test
and -m i used to include and exclude pytest markers...

from pytest --help:

-k EXPRESSION         only run tests which match the given substring expression. An expression is a python evaluatable expression where all names are substring-matched against test names and their parent classes. Example: -k 'test_method
                      or test_other' matches all test functions and classes whose name contains 'test_method' or 'test_other', while -k 'not test_method' matches those that don't contain 'test_method' in their names. -k 'not test_method
                      and not test_other' will eliminate the matches. Additionally keywords are matched to classes and functions containing extra names in their 'extra_keyword_matches' set, as well as functions which have names assigned
                      directly to them. The matching is case-insensitive.
-m MARKEXPR           only run tests matching given mark expression.
                      For example: -m 'mark1 and not mark2'.

which seem to go my way?

vlorentz added inline comments.Apr 9 2021, 10:22 AM

tox.ini
32	I don't understand. Are you saying I should add a marker?

ardumont added inline comments.Apr 9 2021, 10:46 AM

tox.ini
32	No, i'm asking whether it's correct?

It seems to behave mostly ok with what you did so probably "nevermind on what i said earlier".

I have some issues when running it though (after arc patching):

$ tox -e identify-base
...
.tox/identify-base/lib/python3.7/site-packages/swh/model/tests/test_cli.py .............                                                                                                                                                [100%]

====================================================================================================== 13 passed, 1 deselected in 0.09s =======================================================================================================
identify-base run-test: commands[1] | /usr/bin/env bash -c '  ! swh-identify -t snapshot . 2>&1  | (! grep '"'"'Dulwich package'"'"')  > /dev/null '
ERROR: InvocationError for command /usr/bin/env bash -c '  ! swh-identify -t snapshot . 2>&1  | (! grep '"'"'Dulwich package'"'"')  > /dev/null ' (exited with code 1)
___________________________________________________________________________________________________________________ summary ___________________________________________________________________________________________________________________
ERROR:   identify-base: commands failed

this diff depends on D5466

ah but yeah, thx... Undoubtedly, it's one of those days i should have stayed in my bed...

Jenkins runs the py3 tox environment explicitly, so this won't get run by CI. The same issue exists with the previously added "identify" environment.

Please see the swh-core tox.ini for a better (?) way of doing this and having it picked up by Jenkins.

As for the actual test, rather than use a bash script, you should be able to mock __builtins__.__import__ to trigger an ImportError on importing dulwich, rather than have a whole environment dedicated to this.

rebase + rewrite based on @olasd's suggestions

Build is green

Patch application report for D5467 (id=19578)

Could not rebase; Attempt merge onto eeedac7af7...

Updating eeedac7..e7bbfd0
Fast-forward
 pytest.ini                  |  1 +
 requirements-test.txt       |  1 -
 setup.py                    |  1 +
 swh/model/cli.py            | 18 ++++++++++++++++--
 swh/model/tests/test_cli.py | 15 +++++++++++++++
 tox.ini                     | 22 +++++++++++-----------
 6 files changed, 44 insertions(+), 14 deletions(-)

Changes applied before test

commit e7bbfd04d7940996938e680b2d808cc4fd10a2e7
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Apr 8 19:18:10 2021 +0200

    tox: Check swh-identify can run even if Dulwich isn't installed

commit c62f13fd55739b73245cbfa75f0325df4de12957
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Apr 8 18:55:01 2021 +0200

    swh-identify: Hide tracebacks if Click or Dulwich is not installed
    
    And show nice human-readable errors instead

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

Harbormaster completed remote builds in B20611: Diff 19578.Apr 9 2021, 3:44 PM

vlorentz edited the summary of this revision. (Show Details)Apr 12 2021, 1:17 PM

olasd accepted this revision.Apr 12 2021, 1:18 PM

This revision is now accepted and ready to land.Apr 12 2021, 1:18 PM

run py3-minimal after py3-full, so it does not overwrite the coverage

Build is green

Patch application report for D5467 (id=19600)

Could not rebase; Attempt merge onto eeedac7af7...

Updating eeedac7..27a05d6
Fast-forward
 pytest.ini                  |  1 +
 requirements-test.txt       |  1 -
 setup.py                    |  1 +
 swh/model/cli.py            | 18 ++++++++++++++++--
 swh/model/tests/test_cli.py | 15 +++++++++++++++
 tox.ini                     | 22 +++++++++++-----------
 6 files changed, 44 insertions(+), 14 deletions(-)

Changes applied before test

commit 27a05d687f845029e5f8b35a84a63bb7f0e877ff
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Apr 8 19:18:10 2021 +0200

    tox: Check swh-identify can run even if Dulwich isn't installed

commit c62f13fd55739b73245cbfa75f0325df4de12957
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Apr 8 18:55:01 2021 +0200

    swh-identify: Hide tracebacks if Click or Dulwich is not installed
    
    And show nice human-readable errors instead

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

Harbormaster completed remote builds in B20632: Diff 19600.Apr 12 2021, 1:21 PM

Closed by commit rDMOD27a05d687f84: tox: Check swh-identify can run even if Dulwich isn't installed (authored by vlorentz). · Explain WhyApr 12 2021, 1:25 PM

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDMOD27a05d687f84: tox: Check swh-identify can run even if Dulwich isn't installed.

tox: Check swh-identify can run even if Dulwich isn't installed
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D5467 (id=19536)

Changes applied before test

Patch application report for D5467 (id=19540)

Changes applied before test

Patch application report for D5467 (id=19578)

Changes applied before test

Patch application report for D5467 (id=19600)

Changes applied before test

Revision Contents
Changeset List

Diff 19603

pytest.ini

requirements-test.txt

setup.py

swh/model/tests/test_cli.py

tox.ini

tox: Check swh-identify can run even if Dulwich isn't installedClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D5467 (id=19536)

Changes applied before test

Patch application report for D5467 (id=19540)

Changes applied before test

Patch application report for D5467 (id=19578)

Changes applied before test

Patch application report for D5467 (id=19600)

Changes applied before test

Revision ContentsChangeset List

Diff 19603

pytest.ini

requirements-test.txt

setup.py

swh/model/tests/test_cli.py

tox.ini

tox: Check swh-identify can run even if Dulwich isn't installed
ClosedPublic
Actions

Revision Contents
Changeset List