Page MenuHomeSoftware Heritage

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

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

Details

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

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.

ardumont added a project: Journal.

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)

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 edited the summary of this revision. (Show Details)

Fix according to review \m/

swh/journal/pytest_plugin.py
98

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

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.

Use pprint_key function as suggested in irc ;)

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.

  • Rework commit message
  • Reuse same string format to reduce diff stat change

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.

This revision is now accepted and ready to land.Jun 9 2020, 4:21 PM