Page MenuHomeSoftware Heritage

test_client: Move redundant tests setup into fixtures
ClosedPublic

Authored by ardumont on Oct 12 2020, 8:12 AM.

Diff Detail

Event Timeline

Build is green

Patch application report for D4227 (id=14916)

Could not rebase; Attempt merge onto b18cd89d4c...

Updating b18cd89d..0fe94348
Fast-forward
 swh/deposit/cli/client.py                          |  27 +-
 swh/deposit/tests/cli/test_client.py               | 488 ++++++++++++++-------
 .../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, 387 insertions(+), 165 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 0fe94348f60dcf2c748b95f072e502bf4d4eeab9
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 11 12:13:04 2020 +0200

    test_client: Move redundant tests setup into fixtures

commit 2150833f440605c5c48b9fef0369f2ccd752f046
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 10 08:13:58 2020 +0200

    test_client: Explicit the possible format outputs

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/206/ for more details.

vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/deposit/tests/cli/test_client.py
44

it's really unclear from the name of the fixture that it patches tempfile.TemporaryDirectory

This revision now requires changes to proceed.Oct 12 2020, 3:26 PM
swh/deposit/tests/cli/test_client.py
44

does patched_tmp_path sound better ?

Rename tmp_path to patched_tmp_path
really no idea if it's better or not ¯\_(ツ)_/¯

Build has FAILED

Patch application report for D4227 (id=14956)

Could not rebase; Attempt merge onto 419c1b26d0...

Updating 419c1b26..2b164ac1
Fast-forward
 swh/deposit/cli/client.py            |  27 ++-
 swh/deposit/tests/cli/test_client.py | 312 ++++++++++++++++++++++++-----------
 2 files changed, 228 insertions(+), 111 deletions(-)
Changes applied before test
commit 2b164ac19bbfc6902dac19c17eea2dddb139d75c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 11 12:13:04 2020 +0200

    test_client: Move redundant tests setup into fixtures

commit 69e527dc239ad9f07d371d124af7b857aba138cc
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sat Oct 10 08:13:58 2020 +0200

    test_client: Explicit the possible format outputs

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

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

swh/deposit/tests/cli/test_client.py
49

tmp_path here ;)

With the actual fix, it's better

Build has FAILED

Patch application report for D4227 (id=14970)

Rebasing onto 0e0b342d9b...

Current branch diff-target is up to date.
Changes applied before test
commit e22b900cd11d33b41baa49ccaaf6a4711576184c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 11 12:13:04 2020 +0200

    test_client: Move redundant tests setup into fixtures

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

Build is green

Patch application report for D4227 (id=14971)

Rebasing onto 0e0b342d9b...

Current branch diff-target is up to date.
Changes applied before test
commit 255d689569ffa5302cc90ad508eb966b5a8a7f8c
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Sun Oct 11 12:13:04 2020 +0200

    test_client: Move redundant tests setup into fixtures

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

ardumont added inline comments.
swh/deposit/tests/cli/test_client.py
44

i gave it a shot, nothing else comes to mind...

This revision is now accepted and ready to land.Oct 13 2020, 10:26 AM