Page MenuHomeSoftware Heritage
Differential D5564

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

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

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T3279: Fix some invalid resolved browse URLs for SWHIDs with revision anchor
Commits
rDWAPPS1ca5105c4e87: common/identifiers: Fix content SWHID with anchor revision browse URL
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 20967
Build 32539: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 32538: arc lint + arc unit

Event Timeline

anlambert created this revision.Apr 20 2021, 4:57 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptApr 20 2021, 4:57 PM
swh-public-ci added a comment.Apr 20 2021, 5:05 PM

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!
swh-public-ci added a comment.Apr 20 2021, 5:18 PM

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.

Harbormaster completed remote builds in B20885: Diff 19874.Apr 20 2021, 5:18 PM
anlambert requested review of this revision.Apr 20 2021, 5:18 PM
vlorentz added a subscriber: vlorentz.Apr 20 2021, 6:31 PM

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

anlambert added a comment.EditedApr 20 2021, 6:37 PM
In D5564#141188, @vlorentz wrote:

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.

anlambert updated this revision to Diff 19892.Apr 21 2021, 11:47 AM

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

swh-public-ci added a comment.Apr 21 2021, 11:56 AM

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.

Harbormaster completed remote builds in B20900: Diff 19892.Apr 21 2021, 11:56 AM
vlorentz requested changes to this revision.Apr 22 2021, 11:12 AM
vlorentz added inline comments.
swh/web/tests/common/test_identifiers.py
701–710

still not covered

This revision now requires changes to proceed.Apr 22 2021, 11:12 AM
anlambert updated this revision to Diff 19961.Apr 23 2021, 12:23 PM

Rebase and improve tests.

swh-public-ci added a comment.Apr 23 2021, 12:31 PM

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.

Harbormaster completed remote builds in B20963: Diff 19961.Apr 23 2021, 12:31 PM
vlorentz accepted this revision.Apr 23 2021, 12:37 PM

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
anlambert added a comment.Apr 23 2021, 1:01 PM
In D5564#141843, @vlorentz wrote:

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.

anlambert updated this revision to Diff 19965.Apr 23 2021, 1:03 PM

Rebase

swh-public-ci added a comment.Apr 23 2021, 1:10 PM

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.

Harbormaster completed remote builds in B20967: Diff 19965.Apr 23 2021, 1:10 PM
Closed by commit rDWAPPS1ca5105c4e87: common/identifiers: Fix content SWHID with anchor revision browse URL (authored by anlambert). · Explain WhyApr 23 2021, 1:15 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDWAPPS1ca5105c4e87: common/identifiers: Fix content SWHID with anchor revision browse URL.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
web/
common/
9 lines99%
Loading...
tests/
common/
89 lines99%
Loading...
24 lines96%
Loading...

Diff 19965

View Options

swh/web/common/identifiers.py

Loading...
View Options

swh/web/tests/common/test_identifiers.py

Loading...
View Options

swh/web/tests/strategies.py

Loading...
About · Developer information · Legal