Page MenuHomeSoftware Heritage

common/identifiers: Fix content SWHID with anchor revision browse URL
ClosedPublic

Authored by anlambert on Apr 20 2021, 4:57 PM.

Details

Summary

An invalid value was generated for the path query parameter of the
revision view, leading to a 404 error.

Related to T3279

Diff Detail

Repository
rDWAPPS Web applications
Branch
fix-cnt-swhid-rev-anchor-resolving
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20885
Build 32411: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 32410: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D5564 (id=19874)

Rebasing onto 4a6d884a74...

Current branch diff-target is up to date.
Changes applied before test
commit d733ae37eb5e20051526f4cf117667718bd3a1a4
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Related to T3279

Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/700/
See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/700/console

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 20 2021, 5:05 PM
Harbormaster failed remote builds in B20885: Diff 19874!

Build is green

Patch application report for D5564 (id=19874)

Rebasing onto 4a6d884a74...

Current branch diff-target is up to date.
Changes applied before test
commit d733ae37eb5e20051526f4cf117667718bd3a1a4
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Related to T3279

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

Why use hypothesis for this? According to Jenkins, one of the two important branches are not covered.

Why use hypothesis for this? According to Jenkins, one of the two important branches are not covered.

It seems a test is missing for that special case that should only occurs for a SWHID without origin and visit qualifier and a non anchor revision, for instance:
swh:1:cnt:546792d176a4801b7b4f7659ec3953a948a85ba0;anchor=swh:1:dir:94c2b67787a2a40c08831b5a61eb31b7fce267c4;path=/hexagon/kernel/dma.c

Let's add it before landing this then.

Rebase and add tests for content/directory SWHID with directory anchor (missing execution branch to cover).

Build is green

Patch application report for D5564 (id=19892)

Rebasing onto 2f83416d9f...

Current branch diff-target is up to date.
Changes applied before test
commit 461aa57ec4e83a281e5cca9e7533990a954bc93e
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Related to T3279

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

vlorentz added inline comments.
swh/web/tests/common/test_identifiers.py
700–709

still not covered

This revision now requires changes to proceed.Apr 22 2021, 11:12 AM

Build is green

Patch application report for D5564 (id=19961)

Rebasing onto 3a0ffc0879...

Current branch diff-target is up to date.
Changes applied before test
commit 82bcc274fc799acb1602ecd92cc8d34f68cf5fe4
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Also add missing tests for some SWHIDs resolving.
    
    Related to T3279

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

Thanks. I still don't get why we use Hypothesis for this though.

This revision is now accepted and ready to land.Apr 23 2021, 12:37 PM

Thanks. I still don't get why we use Hypothesis for this though.

Because the swh-web tests need to use objects stored into its sample archive and not fake ones.

If you look at the code of resolve_swhid, you will see some checks are done on objects existence,
for instance to ensure an anchor really exists into the archive.

Build is green

Patch application report for D5564 (id=19965)

Rebasing onto 00cc0c4dae...

Current branch diff-target is up to date.
Changes applied before test
commit 1ca5105c4e8748c4efb80a1ed0c67f4830d0039a
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Tue Apr 20 16:54:01 2021 +0200

    common/identifiers: Fix content SWHID with anchor revision browse URL
    
    An invalid value was generated for the path query parameter of the
    revision view, leading to a 404 error.
    
    Also add missing tests for some SWHIDs resolving.
    
    Related to T3279

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