Page MenuHomeSoftware Heritage

loader.mercurial: Increase coverage
ClosedPublic

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

Diff Detail

Repository
rDLDHG Mercurial loader
Branch
improve-coverage
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2980
Build 3806: tox-on-jenkinsJenkins
Build 3805: arc lint + arc unit

Event Timeline

ardumont created this revision.Dec 11 2018, 8:01 PM
ardumont marked an inline comment as done.Dec 11 2018, 8:04 PM
ardumont added inline comments.
swh/loader/mercurial/loader.py
61

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

ardumont updated this revision to Diff 2556.Dec 11 2018, 9:39 PM
  • 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
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/tests/test_loader_verifier.py
192–217

Shouldn't we query the storage instead?

This revision is now accepted and ready to land.Dec 12 2018, 11:56 AM
anlambert accepted this revision.Dec 12 2018, 12:01 PM
anlambert added a subscriber: anlambert.

Apart a couple of nitpicks, looks good to me.

swh/loader/mercurial/tests/test_loader.py
193

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.

swh/loader/mercurial/tests/test_loader_verifier.py
168

This comment is strange.

ardumont marked 3 inline comments as done.Dec 12 2018, 12:09 PM
ardumont added inline comments.
swh/loader/mercurial/tests/test_loader.py
193

Right.

The I stands for integration.

swh/loader/mercurial/tests/test_loader_verifier.py
168

Typo!

192–217

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.

ardumont updated this revision to Diff 2557.Dec 12 2018, 12:11 PM
  • test_loader_verifier: Fix typo
  • tests: Rename ITest to Test classes
This revision was automatically updated to reflect the committed changes.