Page MenuHomeSoftware Heritage
Differential D3249

pytest_plugin: Fix cascading assertion error when key is not hashable
ClosedPublic
Actions

Authored by ardumont on Jun 9 2020, 12:51 PM.

Details

Reviewers
vlorentz
douardda
olasd
Group Reviewers
Reviewers
Commits
rDJNLc91a4cff84a0: pytest_plugin: pprint key when assertion is not respected
Summary

I have issues fixing D3238 and this revealed itself ;)

Without this commit, we have cascading type error when the assertion is not
respected:

*** TypeError: unhashable type: 'dict'

As for example, origin-visit, origin-visit-status,... keys are dict and not
bytes.

Test Plan

tox

Diff Detail

Repository
rDJNL Journal infrastructure
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.Jun 9 2020, 12:51 PM
swh-public-ci added a comment.Jun 9 2020, 12:53 PM

Build is green

Patch application report for D3249 (id=11512)

Rebasing onto d18c5ef32f...

Current branch diff-target is up to date.
Changes applied before test
commit 48b22350d4f80d551b2560a89efe8cf0e4db58bb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Jun 9 12:50:44 2020 +0200

    pytest_plugin: Fix cascading assertion error when key is not hashable

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

Harbormaster completed remote builds in B12716: Diff 11512.Jun 9 2020, 12:53 PM
ardumont edited the summary of this revision. (Show Details)Jun 9 2020, 1:38 PM
ardumont added a project: Journal.
vlorentz added a comment.Jun 9 2020, 1:47 PM

checking for dict instance seems completely arbitrary, and doesn't catch other non-hashable types.

I suggest this instead:

try:
    key_str = hash_to_hex(key)
except TypeError:
    key_str = repr(key)
ardumont added a comment.Jun 9 2020, 2:14 PM

checking for dict instance seems completely arbitrary, and doesn't catch other non-hashable types.

I suggest this instead:

try:
    key_str = hash_to_hex(key)
except TypeError:
    key_str = repr(key)

Sure it is, i did not look up much, went straight to fix so i can actually fix
my initial problem.

Although, i'm pretty sure we only have bytes, str or dict
as key (hash_to_key already being able to be a noop when it's a str).

I'll adapt accordingly nonetheless.

ardumont updated this revision to Diff 11514.Jun 9 2020, 2:16 PM
ardumont edited the summary of this revision. (Show Details)

Fix according to review \m/

ardumont added inline comments.Jun 9 2020, 2:18 PM
swh/journal/pytest_plugin.py
98

(forgot to submit) or we can use {key!r} (in the diff change above) as in here?

swh-public-ci added a comment.Jun 9 2020, 2:18 PM

Build is green

Patch application report for D3249 (id=11514)

Rebasing onto d18c5ef32f...

Current branch diff-target is up to date.
Changes applied before test
commit 4d9b888860f3e42c74663909c96833171d675cb5
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Jun 9 12:50:44 2020 +0200

    pytest_plugin: Fix cascading assertion error when key is not hashable

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

Harbormaster completed remote builds in B12718: Diff 11514.Jun 9 2020, 2:18 PM
ardumont updated this revision to Diff 11519.Jun 9 2020, 4:08 PM

Use pprint_key function as suggested in irc ;)

swh-public-ci added a comment.Jun 9 2020, 4:10 PM

Build is green

Patch application report for D3249 (id=11519)

Rebasing onto d18c5ef32f...

Current branch diff-target is up to date.
Changes applied before test
commit 7dc73c4324827932d8570332f95f98abed34511b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Jun 9 12:50:44 2020 +0200

    pytest_plugin: pprint key when assertion is not respected
    
    This fixes a cascading assertion error when key is not hashable.

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

Harbormaster completed remote builds in B12723: Diff 11519.Jun 9 2020, 4:10 PM
ardumont updated this revision to Diff 11520.Jun 9 2020, 4:14 PM
  • Rework commit message
  • Reuse same string format to reduce diff stat change
swh-public-ci added a comment.Jun 9 2020, 4:15 PM

Build is green

Patch application report for D3249 (id=11520)

Rebasing onto d18c5ef32f...

Current branch diff-target is up to date.
Changes applied before test
commit c91a4cff84a02fc899543bb7711e34748ce659f4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Tue Jun 9 12:50:44 2020 +0200

    pytest_plugin: pprint key when assertion is not respected
    
    Prior to this commit, when error happened during testing, this would cascade
    into another error ("unhashable type dict" for example). In effect, preventing
    to actually understand correctly what went wrong in the first place.
    
    This commit fixes it.

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

Harbormaster completed remote builds in B12724: Diff 11520.Jun 9 2020, 4:15 PM
olasd accepted this revision.Jun 9 2020, 4:21 PM
This revision is now accepted and ready to land.Jun 9 2020, 4:21 PM
Closed by commit rDJNLc91a4cff84a0: pytest_plugin: pprint key when assertion is not respected (authored by ardumont). · Explain WhyJun 9 2020, 4:23 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDJNLc91a4cff84a0: pytest_plugin: pprint key when assertion is not respected.

Revision Contents
Changeset List

PathSize
swh/
journal/
5 lines

Diff 11521

View Options

swh/journal/pytest_plugin.py

Loading...
About · Developer information · Legal