Page MenuHomeSoftware Heritage

phabricator: Add test for new lister implementation
ClosedPublic

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

Details

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

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!

Update: Rebase and fix tests

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.

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.

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 inline comments.
swh/lister/phabricator/tests/test_lister.py
14–15

you could use your fixture here phabricator_repositories_page1

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?

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.

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.

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.

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.