Page MenuHomeSoftware Heritage

ContentLoader: Allow nar computation checks
ClosedPublic

Authored by ardumont on Oct 6 2022, 1:03 PM.

Details

Summary

"nar" computation checks can happen on files too.

This also deduplicates tests code on content and directory ones.

Related to T3781

Related to P1489

Diff Detail

Repository
rDLDBASE Generic VCS/Package Loader
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 D8636 (id=31181)

Rebasing onto 8aa6dab72a...

Current branch diff-target is up to date.
Changes applied before test
commit 72406e67d571bf77c3b38278429010ade07f2ccb
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 6 12:22:45 2022 +0200

    ContentLoader: Allow nar computation checks
    
    "nar" computation checks can happen on files too.
    
    This also deduplicate tests code on content and directory ones.
    
    Related to T3781

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

ardumont retitled this revision from wip: ContentLoader: Allow nar computation checks to ContentLoader: Allow nar computation checks.Oct 6 2022, 3:01 PM
ardumont edited the summary of this revision. (Show Details)
anlambert added a subscriber: anlambert.

Looks good to me, I added a couple of nitpick comments.

swh/loader/core/tests/conftest.py
36–57

I would rather put this function in swh.loader.core.utils module as it could be useful elsewhere, also I think adding a tarball boolean parameter instead of checking extension would make it more generic.

swh/loader/core/tests/test_loader.py
611–620

reason="requires nix-store executable from nix binaries"

796

ditto

This revision is now accepted and ready to land.Oct 7 2022, 1:53 PM
swh/loader/core/tests/conftest.py
36–57

It was not generic because it's a test utils (using the nix_hashes function from the module you mention).

But yes, if we move it as you said, fine, that's gonna how it will end up being implemented.

Although, if we do move it, short of hard-coding some hashes (or mock), i don't really know how to test it.

swh/loader/core/tests/conftest.py
36–57

Fine for me to let it this way then.

ardumont added inline comments.
swh/loader/core/tests/conftest.py
36–57

I went with hard-coding a hash as a non-reg test. That's how the test is doing it for tarball.

Adapt according to suggestion

Build is green

Patch application report for D8636 (id=31215)

Rebasing onto 8aa6dab72a...

Current branch diff-target is up to date.
Changes applied before test
commit 8babe9656ab2bf7a4c025c4016d023e15ae48af2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 6 12:22:45 2022 +0200

    ContentLoader: Allow nar computation checks
    
    "nar" computation checks can happen on files too.
    
    This also deduplicate tests code on content and directory ones.
    
    Related to T3781

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

Build is green

Patch application report for D8636 (id=31215)

Rebasing onto 8aa6dab72a...

Current branch diff-target is up to date.
Changes applied before test
commit 8babe9656ab2bf7a4c025c4016d023e15ae48af2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 6 12:22:45 2022 +0200

    ContentLoader: Allow nar computation checks
    
    "nar" computation checks can happen on files too.
    
    This also deduplicate tests code on content and directory ones.
    
    Related to T3781

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

Build is green

Patch application report for D8636 (id=31215)

Rebasing onto 8aa6dab72a...

Current branch diff-target is up to date.
Changes applied before test
commit 8babe9656ab2bf7a4c025c4016d023e15ae48af2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 6 12:22:45 2022 +0200

    ContentLoader: Allow nar computation checks
    
    "nar" computation checks can happen on files too.
    
    This also deduplicate tests code on content and directory ones.
    
    Related to T3781

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

Drop fixme (it's not working)

Build is green

Patch application report for D8636 (id=31218)

Rebasing onto 8aa6dab72a...

Current branch diff-target is up to date.
Changes applied before test
commit 028b7c04b9edd8fba65d40e0075897effc2222e6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 6 12:22:45 2022 +0200

    ContentLoader: Allow nar computation checks
    
    "nar" computation checks can happen on files too.
    
    This also deduplicate tests code on content and directory ones.
    
    Related to T3781

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

Build is green

Patch application report for D8636 (id=31218)

Rebasing onto 8aa6dab72a...

Current branch diff-target is up to date.
Changes applied before test
commit 028b7c04b9edd8fba65d40e0075897effc2222e6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 6 12:22:45 2022 +0200

    ContentLoader: Allow nar computation checks
    
    "nar" computation checks can happen on files too.
    
    This also deduplicate tests code on content and directory ones.
    
    Related to T3781

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

Build is green

Patch application report for D8636 (id=31218)

Rebasing onto 8aa6dab72a...

Current branch diff-target is up to date.
Changes applied before test
commit 028b7c04b9edd8fba65d40e0075897effc2222e6
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 6 12:22:45 2022 +0200

    ContentLoader: Allow nar computation checks
    
    "nar" computation checks can happen on files too.
    
    This also deduplicate tests code on content and directory ones.
    
    Related to T3781

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

Thanks!

Now i'm happy to merge without decreasing coverage!