Page MenuHomeSoftware Heritage
Differential D4222

deposit.client: Clarify cli error messages
ClosedPublic
Actions

Authored by ardumont on Oct 9 2020, 4:22 PM.

Details

Reviewers
None
Group Reviewers
Reviewers
Commits
rDDEP8e99386fa17d: deposit.client: Improve cli error messages and add missing coverage
Summary

As discussed earlier

This also adds more coverage on the corner cases of the same impacted function.

Test Plan

tox

Diff Detail

Repository
rDDEP Push deposit
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 16137
Build 24825: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 24824: arc lint + arc unit

Event Timeline

ardumont created this revision.Oct 9 2020, 4:22 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptOct 9 2020, 4:22 PM
swh-public-ci added a comment.Oct 9 2020, 4:25 PM

Build is green

Patch application report for D4222 (id=14901)

Could not rebase; Attempt merge onto b18cd89d4c...

Updating b18cd89d..fac8ab15
Fast-forward
 swh/deposit/cli/client.py                          |  18 +-
 swh/deposit/tests/cli/test_client.py               | 231 ++++++++++++++-------
 .../https_deposit.test.metadata/1_test_666_media   |   1 +
 .../https_deposit.test.status/1_servicedocument    |  26 +++
 .../https_deposit.test.status/1_test_1033_status   |  10 +
 5 files changed, 197 insertions(+), 89 deletions(-)
 create mode 120000 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_media
 create mode 100644 swh/deposit/tests/data/https_deposit.test.status/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.test.status/1_test_1033_status
Changes applied before test
commit fac8ab159a52be8395829836f39ddbad51da9314
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 16:23:17 2020 +0200

    deposit.client: Clarify cli error message

commit 419c1b26d0452bc9965ed01c29ce6c1ab5c50831
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 16:10:21 2020 +0200

    test_client: Add a step to update the partial deposit with archive
    
    This should actually cover all the missing cases now.

commit 1a782449064a4228af12ad768862611cc5a49a69
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 13:48:49 2020 +0200

    test_client: Add scenario on deposit status cli
    
    This also checks the 3 different format outputs which were not so far.

commit 404f2728ad0f85789b0827025eafb1c8b6124c05
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 12:17:58 2020 +0200

    test_client: Refactor and simplify tests for a better readability
    
    This:
    - Align all tests to use json format so the output check is simpler
    - Simplify the failure cases to check the caplog record
    - Drop dead code
    - Rename tests with a `test_cli_` prefix so it's simpler to run separately from
    the rest

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

Harbormaster completed remote builds in B16134: Diff 14901.Oct 9 2020, 4:25 PM
ardumont updated this revision to Diff 14904.Oct 9 2020, 5:01 PM

Add missing scenario checks

swh-public-ci added a comment.Oct 9 2020, 5:04 PM

Build is green

Patch application report for D4222 (id=14904)

Could not rebase; Attempt merge onto b18cd89d4c...

Updating b18cd89d..f45383e5
Fast-forward
 swh/deposit/cli/client.py                          |  18 +-
 swh/deposit/tests/cli/test_client.py               | 391 +++++++++++++--------
 .../https_deposit.test.metadata/1_test_666_media   |   1 +
 .../https_deposit.test.status/1_servicedocument    |  26 ++
 .../https_deposit.test.status/1_test_1033_status   |  10 +
 5 files changed, 299 insertions(+), 147 deletions(-)
 create mode 120000 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_media
 create mode 100644 swh/deposit/tests/data/https_deposit.test.status/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.test.status/1_test_1033_status
Changes applied before test
commit f45383e544dd9e6e672cef8646c8a251a12b1c28
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 16:23:17 2020 +0200

    deposit.client: Improve cli error message
    
    And add missing checks

commit 419c1b26d0452bc9965ed01c29ce6c1ab5c50831
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 16:10:21 2020 +0200

    test_client: Add a step to update the partial deposit with archive
    
    This should actually cover all the missing cases now.

commit 1a782449064a4228af12ad768862611cc5a49a69
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 13:48:49 2020 +0200

    test_client: Add scenario on deposit status cli
    
    This also checks the 3 different format outputs which were not so far.

commit 404f2728ad0f85789b0827025eafb1c8b6124c05
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 12:17:58 2020 +0200

    test_client: Refactor and simplify tests for a better readability
    
    This:
    - Align all tests to use json format so the output check is simpler
    - Simplify the failure cases to check the caplog record
    - Drop dead code
    - Rename tests with a `test_cli_` prefix so it's simpler to run separately from
    the rest

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

Harbormaster completed remote builds in B16137: Diff 14904.Oct 9 2020, 5:04 PM
vlorentz added subscribers: moranegg, vlorentz.Oct 9 2020, 5:26 PM

LGTM. @moranegg, what do you think?

vlorentz added a comment.Oct 9 2020, 5:26 PM

LGTM. @moranegg, what do you think?

ardumont updated this revision to Diff 14907.Oct 9 2020, 6:31 PM

Add missing checks again

This should cover all checks now

swh-public-ci added a comment.Oct 9 2020, 6:33 PM

Build is green

Patch application report for D4222 (id=14907)

Could not rebase; Attempt merge onto b18cd89d4c...

Updating b18cd89d..ab8fda35
Fast-forward
 swh/deposit/cli/client.py                          |  20 +-
 swh/deposit/tests/cli/test_client.py               | 403 +++++++++++++++------
 .../https_deposit.test.metadata/1_test_666_media   |   1 +
 .../https_deposit.test.status/1_servicedocument    |  26 ++
 .../https_deposit.test.status/1_test_1033_status   |  10 +
 5 files changed, 345 insertions(+), 115 deletions(-)
 create mode 120000 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_media
 create mode 100644 swh/deposit/tests/data/https_deposit.test.status/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.test.status/1_test_1033_status
Changes applied before test
commit ab8fda35c8bc7bc42567f43067bd2073502e37f4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 16:23:17 2020 +0200

    deposit.client: Improve cli error message
    
    This also adds the missing checks scenarios:
    - no actionable command
    - missing --deposit-id when specifying the --replace flag
    - some more incompatible checks command scenario

commit 419c1b26d0452bc9965ed01c29ce6c1ab5c50831
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 16:10:21 2020 +0200

    test_client: Add a step to update the partial deposit with archive
    
    This should actually cover all the missing cases now.

commit 1a782449064a4228af12ad768862611cc5a49a69
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 13:48:49 2020 +0200

    test_client: Add scenario on deposit status cli
    
    This also checks the 3 different format outputs which were not so far.

commit 404f2728ad0f85789b0827025eafb1c8b6124c05
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 12:17:58 2020 +0200

    test_client: Refactor and simplify tests for a better readability
    
    This:
    - Align all tests to use json format so the output check is simpler
    - Simplify the failure cases to check the caplog record
    - Drop dead code
    - Rename tests with a `test_cli_` prefix so it's simpler to run separately from
    the rest

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

Harbormaster completed remote builds in B16140: Diff 14907.Oct 9 2020, 6:33 PM
ardumont added inline comments.Oct 9 2020, 6:35 PM
swh/deposit/cli/client.py
196

meh, i just can add a scenario which mentions both and be done with this...

225

no idea how to reach this... so meh.

250

that was not covered before.

263

i can let it generate the slug, most likely.

ardumont updated this revision to Diff 14909.Oct 10 2020, 8:11 AM

Fix some checks

swh-public-ci added a comment.Oct 10 2020, 8:14 AM

Build is green

Patch application report for D4222 (id=14909)

Could not rebase; Attempt merge onto b18cd89d4c...

Updating b18cd89d..8e99386f
Fast-forward
 swh/deposit/cli/client.py                          |  27 +-
 swh/deposit/tests/cli/test_client.py               | 441 +++++++++++++++------
 .../https_deposit.test.metadata/1_test_666_media   |   1 +
 .../https_deposit.test.status/1_servicedocument    |  26 ++
 .../https_deposit.test.status/1_test_1033_status   |  10 +
 5 files changed, 369 insertions(+), 136 deletions(-)
 create mode 120000 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_media
 create mode 100644 swh/deposit/tests/data/https_deposit.test.status/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.test.status/1_test_1033_status
Changes applied before test
commit 8e99386fa17d22b0ec1eb0fc4a806b233027deca
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 16:23:17 2020 +0200

    deposit.client: Improve cli error messages and add missing coverage
    
    This adds the missing checks on:
    - no actionable command
    - missing --deposit-id when specifying the --replace flag
    - some more incompatible checks command scenario

commit 419c1b26d0452bc9965ed01c29ce6c1ab5c50831
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 16:10:21 2020 +0200

    test_client: Add a step to update the partial deposit with archive
    
    This should actually cover all the missing cases now.

commit 1a782449064a4228af12ad768862611cc5a49a69
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 13:48:49 2020 +0200

    test_client: Add scenario on deposit status cli
    
    This also checks the 3 different format outputs which were not so far.

commit 404f2728ad0f85789b0827025eafb1c8b6124c05
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 12:17:58 2020 +0200

    test_client: Refactor and simplify tests for a better readability
    
    This:
    - Align all tests to use json format so the output check is simpler
    - Simplify the failure cases to check the caplog record
    - Drop dead code
    - Rename tests with a `test_cli_` prefix so it's simpler to run separately from
    the rest

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

Harbormaster completed remote builds in B16142: Diff 14909.Oct 10 2020, 8:14 AM
ardumont retitled this revision from deposit.client: Clarify cli error message to deposit.client: Clarify cli error messages.Oct 10 2020, 8:18 AM
ardumont edited the summary of this revision. (Show Details)
This revision was not accepted when it landed; it landed in state Needs Review.Oct 12 2020, 5:54 PM
Closed by commit rDDEP8e99386fa17d: deposit.client: Improve cli error messages and add missing coverage (authored by ardumont). · Explain Why
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDDEP8e99386fa17d: deposit.client: Improve cli error messages and add missing coverage.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
deposit/
cli/
18 lines88%
Loading...
tests/
cli/
189 lines100%
Loading...

Diff 14904

View Options

swh/deposit/cli/client.py

Loading...
View Options

swh/deposit/tests/cli/test_client.py

Loading...
About · Developer information · Legal