Page MenuHomeSoftware Heritage

Add specific test to the filtering branch function
ClosedPublic

Authored by ardumont on Oct 1 2021, 4:20 PM.

Details

Summary

The function was somehow tested indirectly and it was not clear enough to me what the code did...

Test Plan

tox

Diff Detail

Repository
rDLDG Git loader
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24162
Build 37710: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 37709: arc lint + arc unit

Event Timeline

Build has FAILED

Patch application report for D6391 (id=23244)

Rebasing onto 1d69d2be3b...

Current branch diff-target is up to date.
Changes applied before test
commit fd2de7d2a29c34e129df1ef561c21149e31f6890
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 1 16:19:12 2021 +0200

    Actually drop pull request and merge branches
    
    The actual implementation seemed to be willing to but did not do it.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 1 2021, 4:22 PM
Harbormaster failed remote builds in B24161: Diff 23244!
swh/loader/git/utils.py
89 ↗(On Diff #23244)

I'm confused by this old code... what exactly should that filter out?

swh/loader/git/utils.py
87 ↗(On Diff #23244)

please restore the comment for that condition

swh/loader/git/utils.py
87 ↗(On Diff #23244)

yeah, totally, i'll transform that diff into an improvment one.
as we want to keep the behavior in the end.

89 ↗(On Diff #23244)

Without that diff, we actually have plenty of PR branches, do we want them or do we want to filter them out?

[1] P1191

Transform diff into an extra test

ardumont retitled this revision from Actually drop pull request and merge branches to Add specific test to the filtering branch function.Oct 1 2021, 5:19 PM
ardumont edited the summary of this revision. (Show Details)

Build is green

Patch application report for D6391 (id=23245)

Rebasing onto 1d69d2be3b...

Current branch diff-target is up to date.
Changes applied before test
commit a5fef2202df0f46c35ceafac968395206d1eb985
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 1 16:19:12 2021 +0200

    Add specific test to the filtering branch function

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

vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/loader/git/tests/test_utils.py
51–59

no love for sets? :(

This revision is now accepted and ready to land.Oct 1 2021, 5:24 PM
swh/loader/git/tests/test_utils.py
51–59

you can even use a set comprehension directly instead of set()

swh/loader/git/tests/test_utils.py
51–59

so true ;)

Build is green

Patch application report for D6391 (id=23246)

Rebasing onto 1d69d2be3b...

Current branch diff-target is up to date.
Changes applied before test
commit 8d371e815cc6a46965a288c6bef40500b15c2069
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 1 16:19:12 2021 +0200

    Add specific test to the filtering branch function

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