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 2979
Build 3804: tox-on-jenkinsJenkins
Build 3803: arc lint + arc unit

Event Timeline

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

  • test_loader_verifier: Simplify test initialization
  • loader.mercurial: Add the mercurial archive loader scenario
  • loader.mercurial: Add the mercurial archive loader failure scenario
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 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 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.

  • test_loader_verifier: Fix typo
  • tests: Rename ITest to Test classes
This revision was automatically updated to reflect the committed changes.