Page MenuHomeSoftware Heritage
Differential D4864

phabricator: Add test for new lister implementation
ClosedPublic
Actions

Authored by anlambert on Jan 14 2021, 2:51 PM.

Details

Reviewers
ardumont
olasd
Group Reviewers
Reviewers
Commits
rDLSb743c36496f0: phabricator: Add test for new lister implementation
Summary

Also remove no longer used JSON files.

Depends on D4863

Diff Detail

Repository
rDLS Listers
Branch
phabricator-lister-tests
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18407
Build 28448: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 28447: arc lint + arc unit

Event Timeline

anlambert created this revision.Jan 14 2021, 2:51 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptJan 14 2021, 2:51 PM
swh-public-ci added a comment.Jan 14 2021, 2:56 PM

Build has FAILED

Patch application report for D4864 (id=17229)

Could not rebase; Attempt merge onto c782275296...

Updating c782275..c66d320
Fast-forward
 swh/lister/phabricator/lister.py                   |    4 +-
 .../phabricator/tests/data/api_empty_response.json |    8 -
 .../phabricator/tests/data/api_first_response.json | 2538 --------------------
 .../data/api_first_response_other_instance.json    | 2336 ------------------
 .../phabricator/tests/data/api_next_response.json  | 2354 ------------------
 .../data/api_response_undefined_protocol.json      |   60 -
 ...oldest,attachments[uris]=1,after=,api.token=foo |    1 -
 .../data/phabricator_api_repositories_page1.json   | 1246 ++++++++++
 .../data/phabricator_api_repositories_page2.json   | 1308 ++++++++++
 swh/lister/phabricator/tests/test_lister.py        |   73 +-
 swh/lister/phabricator/tests/test_tasks.py         |    2 +-
 11 files changed, 2629 insertions(+), 7301 deletions(-)
 delete mode 100644 swh/lister/phabricator/tests/data/api_empty_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response_other_instance.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_next_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_response_undefined_protocol.json
 delete mode 120000 swh/lister/phabricator/tests/data/https_forge.softwareheritage.org/api_diffusion.repository.search,order=oldest,attachments[uris]=1,after=,api.token=foo
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page1.json
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page2.json
Changes applied before test
commit c66d32019a23bfc23d8455b1bc3ef8d558979843
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 14:44:32 2021 +0100

    phabricator: Add test for new lister implementation
    
    Also remove no more used JSON files.

commit 85b7e1cd364592da5f7cf171a63beaa321b3d824
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 13:50:48 2021 +0100

    phabricator: Allow to pass forge base URL as lister parameter

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

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 14 2021, 2:56 PM
Harbormaster failed remote builds in B18373: Diff 17229!
anlambert updated this revision to Diff 17231.Jan 14 2021, 3:12 PM

Update: Rebase and fix tests

swh-public-ci added a comment.Jan 14 2021, 3:15 PM

Build is green

Patch application report for D4864 (id=17231)

Could not rebase; Attempt merge onto c782275296...

Updating c782275..81863ad
Fast-forward
 swh/lister/phabricator/lister.py                   |    6 +-
 .../phabricator/tests/data/api_empty_response.json |    8 -
 .../phabricator/tests/data/api_first_response.json | 2538 --------------------
 .../data/api_first_response_other_instance.json    | 2336 ------------------
 .../phabricator/tests/data/api_next_response.json  | 2354 ------------------
 .../data/api_response_undefined_protocol.json      |   60 -
 ...oldest,attachments[uris]=1,after=,api.token=foo |    1 -
 .../data/phabricator_api_repositories_page1.json   | 1246 ++++++++++
 .../data/phabricator_api_repositories_page2.json   | 1308 ++++++++++
 swh/lister/phabricator/tests/test_lister.py        |   94 +-
 swh/lister/phabricator/tests/test_tasks.py         |    2 +-
 11 files changed, 2646 insertions(+), 7307 deletions(-)
 delete mode 100644 swh/lister/phabricator/tests/data/api_empty_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response_other_instance.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_next_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_response_undefined_protocol.json
 delete mode 120000 swh/lister/phabricator/tests/data/https_forge.softwareheritage.org/api_diffusion.repository.search,order=oldest,attachments[uris]=1,after=,api.token=foo
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page1.json
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page2.json
Changes applied before test
commit 81863ad1789fffbe18b4f70afae31f3451287a5d
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 14:44:32 2021 +0100

    phabricator: Add test for new lister implementation
    
    Also remove no more used JSON files.

commit 9ef56c0aaad5eb5c69296872d61d0ba54ed0cacd
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 13:50:48 2021 +0100

    phabricator: Allow to pass forge base URL as lister parameter

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

Harbormaster completed remote builds in B18375: Diff 17231.Jan 14 2021, 3:15 PM
anlambert requested review of this revision.Jan 14 2021, 3:15 PM
anlambert updated this revision to Diff 17233.Jan 14 2021, 3:17 PM

Rebase

swh-public-ci added a comment.Jan 14 2021, 3:22 PM

Build is green

Patch application report for D4864 (id=17233)

Could not rebase; Attempt merge onto c782275296...

Updating c782275..fbc0203
Fast-forward
 swh/lister/phabricator/lister.py                   |    6 +-
 .../phabricator/tests/data/api_empty_response.json |    8 -
 .../phabricator/tests/data/api_first_response.json | 2538 --------------------
 .../data/api_first_response_other_instance.json    | 2336 ------------------
 .../phabricator/tests/data/api_next_response.json  | 2354 ------------------
 .../data/api_response_undefined_protocol.json      |   60 -
 ...oldest,attachments[uris]=1,after=,api.token=foo |    1 -
 .../data/phabricator_api_repositories_page1.json   | 1246 ++++++++++
 .../data/phabricator_api_repositories_page2.json   | 1308 ++++++++++
 swh/lister/phabricator/tests/test_lister.py        |   94 +-
 swh/lister/phabricator/tests/test_tasks.py         |    2 +-
 11 files changed, 2646 insertions(+), 7307 deletions(-)
 delete mode 100644 swh/lister/phabricator/tests/data/api_empty_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response_other_instance.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_next_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_response_undefined_protocol.json
 delete mode 120000 swh/lister/phabricator/tests/data/https_forge.softwareheritage.org/api_diffusion.repository.search,order=oldest,attachments[uris]=1,after=,api.token=foo
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page1.json
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page2.json
Changes applied before test
commit fbc02034165a82c77891ab20dbd06cc5cca1e682
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 14:44:32 2021 +0100

    phabricator: Add test for new lister implementation
    
    Also remove no more used JSON files.

commit 370a04dbf23a8ecc78af438bb1f066630a7b3d8d
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 13:50:48 2021 +0100

    phabricator: Allow to pass forge base URL as lister parameter

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

Harbormaster completed remote builds in B18377: Diff 17233.Jan 14 2021, 3:22 PM
anlambert added a child revision: D4865: phabricator: Ensure request errors are raised as exceptions.Jan 14 2021, 5:24 PM
ardumont accepted this revision.Jan 14 2021, 7:18 PM
ardumont edited the summary of this revision. (Show Details)
ardumont added a subscriber: ardumont.

lgtm

This revision is now accepted and ready to land.Jan 14 2021, 7:19 PM
tenma added a subscriber: tenma.Jan 15 2021, 10:51 AM
tenma added inline comments.
swh/lister/phabricator/tests/test_lister.py
14–15

you could use your fixture here phabricator_repositories_page1

olasd added a subscriber: olasd.Jan 15 2021, 11:24 AM

Thanks! This is really nice.

I have made a few comments inline, that can be addressed in this diff or as a followup.

swh/lister/phabricator/tests/test_lister.py
14–15

I think this test could be split in two parts, one for the common behavior, one for the legacy api response ("undefined protocol") behavior.

87–93

Would it be possible to check that the API token and user-agent are properly passed for these requests?

anlambert added inline comments.Jan 15 2021, 1:36 PM
swh/lister/phabricator/tests/test_lister.py
14–15

Well seen, I will make the changes.

14–15

ack

87–93

Sure, I guess we could further match mocked requests based on the headers.

anlambert updated this revision to Diff 17263.Jan 15 2021, 1:57 PM

Update: Address @tenma and @olasd comments

swh-public-ci added a comment.Jan 15 2021, 2:00 PM

Build is green

Patch application report for D4864 (id=17263)

Could not rebase; Attempt merge onto c782275296...

Updating c782275..f4fa83a
Fast-forward
 swh/lister/phabricator/lister.py                   |    6 +-
 .../phabricator/tests/data/api_empty_response.json |    8 -
 .../phabricator/tests/data/api_first_response.json | 2538 --------------------
 .../data/api_first_response_other_instance.json    | 2336 ------------------
 .../phabricator/tests/data/api_next_response.json  | 2354 ------------------
 .../data/api_response_undefined_protocol.json      |   60 -
 ...oldest,attachments[uris]=1,after=,api.token=foo |    1 -
 .../data/phabricator_api_repositories_page1.json   | 1246 ++++++++++
 .../data/phabricator_api_repositories_page2.json   | 1308 ++++++++++
 swh/lister/phabricator/tests/test_lister.py        |  112 +-
 swh/lister/phabricator/tests/test_tasks.py         |    2 +-
 11 files changed, 2658 insertions(+), 7313 deletions(-)
 delete mode 100644 swh/lister/phabricator/tests/data/api_empty_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response_other_instance.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_next_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_response_undefined_protocol.json
 delete mode 120000 swh/lister/phabricator/tests/data/https_forge.softwareheritage.org/api_diffusion.repository.search,order=oldest,attachments[uris]=1,after=,api.token=foo
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page1.json
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page2.json
Changes applied before test
commit f4fa83a90777077f07422529882ce2facf2eb98f
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 14:44:32 2021 +0100

    phabricator: Add test for new lister implementation
    
    Also remove no longer used JSON files.

commit 370a04dbf23a8ecc78af438bb1f066630a7b3d8d
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 13:50:48 2021 +0100

    phabricator: Allow to pass forge base URL as lister parameter

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

Harbormaster completed remote builds in B18403: Diff 17263.Jan 15 2021, 2:00 PM
anlambert updated this revision to Diff 17267.Jan 15 2021, 2:16 PM

Rebase

swh-public-ci added a comment.Jan 15 2021, 2:22 PM

Build is green

Patch application report for D4864 (id=17267)

Could not rebase; Attempt merge onto c782275296...

Updating c782275..01072ea
Fast-forward
 swh/lister/phabricator/lister.py                   |   16 +-
 .../phabricator/tests/data/api_empty_response.json |    8 -
 .../phabricator/tests/data/api_first_response.json | 2538 --------------------
 .../data/api_first_response_other_instance.json    | 2336 ------------------
 .../phabricator/tests/data/api_next_response.json  | 2354 ------------------
 .../data/api_response_undefined_protocol.json      |   60 -
 ...oldest,attachments[uris]=1,after=,api.token=foo |    1 -
 .../data/phabricator_api_repositories_page1.json   | 1246 ++++++++++
 .../data/phabricator_api_repositories_page2.json   | 1308 ++++++++++
 swh/lister/phabricator/tests/test_lister.py        |  112 +-
 swh/lister/phabricator/tests/test_tasks.py         |    2 +-
 11 files changed, 2668 insertions(+), 7313 deletions(-)
 delete mode 100644 swh/lister/phabricator/tests/data/api_empty_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response_other_instance.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_next_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_response_undefined_protocol.json
 delete mode 120000 swh/lister/phabricator/tests/data/https_forge.softwareheritage.org/api_diffusion.repository.search,order=oldest,attachments[uris]=1,after=,api.token=foo
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page1.json
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page2.json
Changes applied before test
commit 01072ea7904ebb33964ed06e841a7d8e3985c7fc
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 14:44:32 2021 +0100

    phabricator: Add test for new lister implementation
    
    Also remove no longer used JSON files.

commit c5652905952a55afee31dfc126b73e05534de43d
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 13:50:48 2021 +0100

    phabricator: Allow to pass forge base URL as lister parameter

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

Harbormaster completed remote builds in B18407: Diff 17267.Jan 15 2021, 2:22 PM
anlambert updated this revision to Diff 17303.Jan 18 2021, 12:14 PM

Rebase

swh-public-ci added a comment.Jan 18 2021, 12:20 PM

Build is green

Patch application report for D4864 (id=17303)

Could not rebase; Attempt merge onto d1fbccd988...

Updating d1fbccd..b743c36
Fast-forward
 swh/lister/phabricator/lister.py                   |   16 +-
 .../phabricator/tests/data/api_empty_response.json |    8 -
 .../phabricator/tests/data/api_first_response.json | 2538 --------------------
 .../data/api_first_response_other_instance.json    | 2336 ------------------
 .../phabricator/tests/data/api_next_response.json  | 2354 ------------------
 .../data/api_response_undefined_protocol.json      |   60 -
 ...oldest,attachments[uris]=1,after=,api.token=foo |    1 -
 .../data/phabricator_api_repositories_page1.json   | 1246 ++++++++++
 .../data/phabricator_api_repositories_page2.json   | 1308 ++++++++++
 swh/lister/phabricator/tests/test_lister.py        |  112 +-
 swh/lister/phabricator/tests/test_tasks.py         |    2 +-
 11 files changed, 2668 insertions(+), 7313 deletions(-)
 delete mode 100644 swh/lister/phabricator/tests/data/api_empty_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_first_response_other_instance.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_next_response.json
 delete mode 100644 swh/lister/phabricator/tests/data/api_response_undefined_protocol.json
 delete mode 120000 swh/lister/phabricator/tests/data/https_forge.softwareheritage.org/api_diffusion.repository.search,order=oldest,attachments[uris]=1,after=,api.token=foo
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page1.json
 create mode 100644 swh/lister/phabricator/tests/data/phabricator_api_repositories_page2.json
Changes applied before test
commit b743c36496f03a38a0b922a1c8e765a873a1c732
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 14:44:32 2021 +0100

    phabricator: Add test for new lister implementation
    
    Also remove no longer used JSON files.

commit d691c04eb80f3f70906fc2e61e19ef057cc312ba
Author: Antoine Lambert <antoine.lambert@inria.fr>
Date:   Thu Jan 14 13:50:48 2021 +0100

    phabricator: Allow to pass forge base URL as lister parameter

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

Harbormaster completed remote builds in B18442: Diff 17303.Jan 18 2021, 12:20 PM
olasd accepted this revision.Jan 18 2021, 5:39 PM
Closed by commit rDLSb743c36496f0: phabricator: Add test for new lister implementation (authored by anlambert). · Explain WhyJan 18 2021, 5:51 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDLSb743c36496f0: phabricator: Add test for new lister implementation.

Revision Contents
Changeset List

Diff 17267

View Options

swh/lister/phabricator/tests/data/api_empty_response.json

Loading...
View Options

swh/lister/phabricator/tests/data/api_first_response.json

Loading...
View Options

swh/lister/phabricator/tests/data/api_first_response_other_instance.json

Loading...
View Options

swh/lister/phabricator/tests/data/api_next_response.json

Loading...
View Options

swh/lister/phabricator/tests/data/api_response_undefined_protocol.json

Loading...
View Options

swh/lister/phabricator/tests/data/https_forge.softwareheritage.org/api_diffusion.repository.search,order=oldest,attachments[uris]=1,after=,api.token=foo

Loading...
View Options

swh/lister/phabricator/tests/data/phabricator_api_repositories_page1.json

Loading...
View Options

swh/lister/phabricator/tests/data/phabricator_api_repositories_page2.json

Loading...
View Options

swh/lister/phabricator/tests/test_lister.py

Loading...
About · Developer information · Legal