Differential D3516

Fix test_loader's "id" types for proper bytes
ClosedPublic
Actions

Authored by douardda on Jul 16 2020, 9:51 AM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Commits

rDLDHG786f8caa37af: Fix test_loader's "id" types for proper bytes

Summary

instead of hex string representation.

Test Plan

This should pass with current swh.loader.core version as well as with D3501.

Diff Detail

Repository

rDLDHG Mercurial loader

Branch

master

Lint

Lint Skipped

Unit

Unit Tests Skipped

Build Status

Buildable 13653
Build 20917: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 20916: arc lint + arc unit

Event Timeline

douardda created this revision.Jul 16 2020, 9:51 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptJul 16 2020, 9:51 AM

Build is green

Patch application report for D3516 (id=12426)

Rebasing onto eb613d754d...

Current branch diff-target is up to date.

Changes applied before test

commit fc82f956355c59770a0500468730e6982520aa5e
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jul 16 09:50:03 2020 +0200

    Fix test_loader's "id" types for proper bytes
    
    instead of hex string representation.

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

Harbormaster completed remote builds in B13647: Diff 12426.Jul 16 2020, 9:53 AM

douardda edited the test plan for this revision. (Show Details)Jul 16 2020, 9:55 AM

douardda mentioned this in D3501: Use bytes as snapshot id and get rid of dict manipulation in check_snapshot().

I'm not sure we still need the hex form now.

ok nonetheless ;)

This revision is now accepted and ready to land.Jul 16 2020, 9:57 AM

ardumont mentioned this in D3517: Fix test_from_disk's snapshot "id" type for bytes.Jul 16 2020, 9:59 AM

In D3516#86518, @ardumont wrote:

I'm not sure we still need the hex form now.

ok nonetheless ;)

It's easier to read :-)

In D3516#86529, @douardda wrote:

In D3516#86518, @ardumont wrote:

I'm not sure we still need the hex form now.

ok nonetheless ;)

It's easier to read :-)

Yes sure, i meant it from:

"id": hash_to_bytes(expected_snapshot_id),

to:

"id: hash_to_bytes("d35668e02e2ba4321dc951cd308cf883786f918a"),

(and drop expected_snapshot_id if it's no longer used which i think it's the case now).

Yes sure, i meant it from:
"id": hash_to_bytes(expected_snapshot_id),
to:
"id: hash_to_bytes("d35668e02e2ba4321dc951cd308cf883786f918a"),
(and drop expected_snapshot_id if it's no longer used which i think it's the case now).

of course, i took the wrong path again... (i should have read back the code before answering sorry):

# because it's still used but we no longer need its hex form
expected_snapshot_id = hash_to_bytes("d35668e02e2ba4321dc951cd308cf883786f918a"),
...
"id: expected_snapshot_id,
...

ohh... it's unreadable, i'll target the diff instead... (sorry)

ardumont added inline comments.Jul 16 2020, 10:07 AM

swh/loader/mercurial/tests/test_loader.py
136–137	expected_snapshot_id = hash_to_bytes("d35668e02e2ba4321dc951cd308cf883786f918a")
138	"id": expected_snapshot_id,
176	snapshot=expected_snapshot_id,

ohh... it's unreadable, i'll target the diff instead... (sorry)

done

(but do as you wish heh, i already accepted it :)

inline expected_snapshot_id

Build is green

Patch application report for D3516 (id=12432)

Rebasing onto eb613d754d...

Current branch diff-target is up to date.

Changes applied before test

commit 786f8caa37af4dec1826dd9a381307dc9b940a68
Author: David Douard <david.douard@sdfa3.org>
Date:   Thu Jul 16 09:50:03 2020 +0200

    Fix test_loader's "id" types for proper bytes
    
    instead of hex string representation.

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

Harbormaster completed remote builds in B13653: Diff 12432.Jul 16 2020, 10:38 AM

even better ;)

thanks!

Closed by commit rDLDHG786f8caa37af: Fix test_loader's "id" types for proper bytes (authored by douardda). · Explain WhyJul 16 2020, 10:39 AM

This revision was automatically updated to reflect the committed changes.

douardda added a commit: rDLDHG786f8caa37af: Fix test_loader's "id" types for proper bytes.