Page MenuHomeSoftware Heritage

deposit.client: Clarify cli error messages
ClosedPublic

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

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Fri, Oct 9, 4:22 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.

ardumont updated this revision to Diff 14904.Fri, Oct 9, 5:01 PM

Add missing scenario checks

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.

LGTM. @moranegg, what do you think?

LGTM. @moranegg, what do you think?

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

Add missing checks again

This should cover all checks now

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.

ardumont added inline comments.Fri, Oct 9, 6:35 PM
swh/deposit/cli/client.py
196

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

230

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

249

that was not covered before.

262

i can let it generate the slug, most likely.

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

Fix some checks

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.

ardumont retitled this revision from deposit.client: Clarify cli error message to deposit.client: Clarify cli error messages.Sat, Oct 10, 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.Mon, Oct 12, 5:54 PM
This revision was automatically updated to reflect the committed changes.