Differential D4231

test_checks: Use pytest.mark.parametrize
ClosedPublic
Actions

Authored by ardumont on Oct 12 2020, 1:49 PM.

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Commits

rDDEP0e0b342d9b65: test_checks: Use pytest.mark.parametrize

Summary

This will allow to be a bit more lenient on those checks (then adding some more sample tests without
the need to craft new tests, see D4232).

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.Oct 12 2020, 1:49 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptOct 12 2020, 1:49 PM

Build is green

Patch application report for D4231 (id=14936)

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

Updating 419c1b26..8861413d
Fast-forward
 swh/deposit/cli/client.py                          |  38 +-
 swh/deposit/client.py                              | 218 +++++++----
 swh/deposit/tests/api/test_checks.py               |  94 +++--
 swh/deposit/tests/cli/test_client.py               | 401 ++++++++++++++++-----
 .../1_servicedocument                              |  26 ++
 .../1_test_123_metadata                            |  10 +
 .../1_test_123_status                              |  10 +
 .../1_test_321_status                              |   8 +
 8 files changed, 566 insertions(+), 239 deletions(-)
 create mode 100644 swh/deposit/tests/data/https_deposit.test.updateswhid/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_123_metadata
 create mode 100644 swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_123_status
 create mode 100644 swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_321_status

Changes applied before test

commit 8861413d52e2e7719b5a70b0bbc9f7876a68891b
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Oct 12 13:49:55 2020 +0200

    test_checks: Use pytest.mark.parametrize
    
    This will allow to improve the existing code and add some more sample without
    the need to craft new tests.

commit 866ec64139b375adb93352414751cffd4d756ea2
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Oct 12 08:11:30 2020 +0200

    deposit_client: Allow deposit metadata update on completed deposit
    
    Related to T2538

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

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

Harbormaster completed remote builds in B16168: Diff 14936.Oct 12 2020, 1:52 PM

vlorentz added a subscriber: vlorentz.Oct 12 2020, 1:55 PM

vlorentz added inline comments.

swh/deposit/tests/api/test_checks.py
14–30	what is the difference in handling between these two?
35–55	that's a little hard to read, with the metadata and errors mixed together :/

ardumont added inline comments.Oct 12 2020, 2:02 PM

swh/deposit/tests/api/test_checks.py
14–30	either name or title needs to be there. The first one provides the name, the other the title. And as they both declare the required author field, their check pass.
35–55	How to do better though? i'm planning on changing the current checks and adding plenty of tests whose body is the same does not excite me much.

ardumont mentioned this in D4232: deposit.api.checks: Update api checks to allow more alternate fields.Oct 12 2020, 2:05 PM

ardumont edited the summary of this revision. (Show Details)Oct 12 2020, 2:05 PM

ardumont edited the summary of this revision. (Show Details)Oct 12 2020, 2:48 PM

ardumont added inline comments.

swh/deposit/tests/api/test_checks.py
35–55	to clarify... ... "and this" does not excite me much to copy/paste those body instructions all around.

vlorentz accepted this revision.Oct 12 2020, 3:04 PM

vlorentz added inline comments.

swh/deposit/tests/api/test_checks.py
14–30	oh right, I missed that name != title
35–55	I don't know. :/

This revision is now accepted and ready to land.Oct 12 2020, 3:04 PM

Rebase
(move the commit down the stack so it can land soon)

Harbormaster completed remote builds in B16196: Diff 14967.Oct 12 2020, 5:58 PM

Use the right repository

Build is green

Patch application report for D4231 (id=14968)

Rebasing onto 69e527dc23...

Current branch diff-target is up to date.

Changes applied before test

commit 0e0b342d9b6505d208052de7cd2a01c864e76edd
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Oct 12 13:49:55 2020 +0200

    test_checks: Use pytest.mark.parametrize
    
    This will allow to improve the existing code and add some more sample without
    the need to craft new tests.

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

Harbormaster completed remote builds in B16197: Diff 14968.Oct 12 2020, 6:01 PM

Closed by commit rDDEP0e0b342d9b65: test_checks: Use pytest.mark.parametrize (authored by ardumont). · Explain WhyOct 12 2020, 6:08 PM

This revision was automatically updated to reflect the committed changes.

ardumont added a commit: rDDEP0e0b342d9b65: test_checks: Use pytest.mark.parametrize.