Paths

Table of Contentst

Differential D802

loader.mercurial: Increase coverage
ClosedPublic
Actions

Authored by ardumont on Dec 11 2018, 8:01 PM.

Tags

None

Subscribers

Details

Reviewers

vlorentz
anlambert

Group Reviewers

Commits

rDLDHG4f89686fdc09: tests: Rename ITest to Test classes
rDLDHG076c5aa959a4: test_loader_verifier: Fix typo
rDLDHG9bb922bef12d: loader.mercurial: Add the mercurial archive loader failure scenario
rDLDHGb0c03268de3d: test_converters: Add coverage on converters
rDLDHGac1e6227d359: loader.mercurial: Add the mercurial archive loader scenario
rDLDHG34c5831bbef4: test_loader_verifier: Simplify test initialization
rDLDHGdd9182a05e13: tests: Add another loader verifier test scenario
rDLDHGca237dc86e2f: tests_tasks: Add scenario to check tasks are fine
rDLDHG19c15e64211e: loader_verifier: Transform the verifier into test
rDLDHG293b7833d254: loader.mercurial: Flip the reduce_effort flag to False
rDLDHG5985c5245a56: loader_verifier: Clear dead comments

Summary

As per my suggestion [1], transformed the loader verifier code into tests.

loader_verifier: Transform the verifier into tests
tests_tasks: Add scenario to check tasks are fine
test_converters: Add coverage on converters
Add scenario for archive mercurial loader (success + failure)

[1] https://forge.softwareheritage.org/T1411#26191

Related T1411
Related T1439
Related T1443

Test Plan

tox

Diff Detail

Repository

rDLDHG Mercurial loader

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.Dec 11 2018, 8:01 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptDec 11 2018, 8:01 PM

Harbormaster completed remote builds in B2978: Diff 2555.Dec 11 2018, 8:03 PM

Build is green
See https://jenkins.softwareheritage.org/job/DLDHG/job/tox/19/ for more details.

ardumont marked an inline comment as done.Dec 11 2018, 8:04 PM

ardumont added inline comments.

swh/loader/mercurial/loader.py
61 ↗	(On Diff #2555)	because it's unused, also not captured in tests [1] I'm not sure i understand fully how it works so... [1] https://forge.softwareheritage.org/source/puppet-swh-site/browse/production/data/defaults.yaml$1847

test_loader_verifier: Simplify test initialization
loader.mercurial: Add the mercurial archive loader scenario
loader.mercurial: Add the mercurial archive loader failure scenario

ardumont edited the summary of this revision. (Show Details)Dec 11 2018, 9:40 PM

Build is green
See https://jenkins.softwareheritage.org/job/DLDHG/job/tox/20/ for more details.

Harbormaster completed remote builds in B2979: Diff 2556.Dec 11 2018, 9:41 PM

ardumont edited the summary of this revision. (Show Details)Dec 12 2018, 10:49 AM

vlorentz accepted this revision.Dec 12 2018, 11:56 AM

vlorentz added a subscriber: vlorentz.

vlorentz added inline comments.

swh/loader/mercurial/loader_verifier.py
186–210	Shouldn't we query the storage instead?

This revision is now accepted and ready to land.Dec 12 2018, 11:56 AM

Apart a couple of nitpicks, looks good to me.

swh/loader/mercurial/loader_verifier.py
162	This comment is strange.
swh/loader/mercurial/tests/test_loader.py
193 ↗	(On Diff #2556)	Small nitpick, I would rename 'LoaderITest2' into 'LoaderTest2' here and do the same for all test class names. I don't see the point of that extra 'I' character.

ardumont marked 3 inline comments as done.Dec 12 2018, 12:09 PM

ardumont added inline comments.

swh/loader/mercurial/loader_verifier.py
162	Typo!
186–210	I do not think so. As we do not call the loader's `load` function, because of history reasons i suppose [1]. We want to call the inner entrypoints here (I think). Then again, I only ported the existing code. In the long run, that might be rewritten or completely dropped. [1] IIRC, when i took other the loader mercurial, that did not work properly with the storage for example.
swh/loader/mercurial/tests/test_loader.py
193 ↗	(On Diff #2556)	Right. The I stands for integration.

test_loader_verifier: Fix typo
tests: Rename ITest to Test classes

Build is green
See https://jenkins.softwareheritage.org/job/DLDHG/job/tox/21/ for more details.

Harbormaster completed remote builds in B2980: Diff 2557.Dec 12 2018, 12:12 PM

Closed by commit rDLDHG5985c5245a56: loader_verifier: Clear dead comments (authored by ardumont). · Explain WhyDec 12 2018, 1:23 PM

This revision was automatically updated to reflect the committed changes.

ardumont edited the summary of this revision. (Show Details)Dec 14 2018, 8:41 AM

ardumont mentioned this in T1443: Make swh services run within docker and docker-compose.

Revision Contents
Changeset List

Path

Size

swh/

loader/

mercurial/

loader_verifier.py

10 lines

Diff 2558

swh/loader/mercurial/loader_verifier.py

Loading...