Page MenuHomeSoftware Heritage

test_client: Refactor and simplify tests for a better readability
ClosedPublic

Authored by ardumont on Oct 9 2020, 12:18 PM.

Details

Summary

This:

  • Aligns all tests to use json format output so the assertion check is simpler (no tampering with caplog record)
  • Simplify the caplog record assertion check in failure scenarios
  • Drop dead code
  • Rename tests with a test_cli_ prefix so it's simpler to run them separately from

the rest

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

Build is green

Patch application report for D4214 (id=14856)

Could not rebase; Attempt merge onto 6b7d5336f7...

Updating 6b7d5336..0e58de44
Fast-forward
 swh/deposit/tests/cli/test_client.py               | 289 +++++++++------------
 swh/deposit/tests/conftest.py                      |  14 +
 .../data/https_deposit.swh.test/1_servicedocument  |  26 ++
 .../tests/data/https_deposit.swh.test/1_test       |  19 ++
 .../https_deposit.test.metadata/1_servicedocument  |  26 ++
 .../tests/data/https_deposit.test.metadata/1_test  |  19 ++
 .../1_test_666_metadata                            |  19 ++
 .../https_deposit.test.metadata/1_test_666_status  |   8 +
 swh/deposit/tests/loader/conftest.py               |  14 -
 9 files changed, 254 insertions(+), 180 deletions(-)
 create mode 100644 swh/deposit/tests/data/https_deposit.swh.test/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.swh.test/1_test
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_metadata
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_status
Changes applied before test
commit 0e58de4473436b9033b619d278298c544d796092
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

commit 91c923ffad090ff5002acfdcb789c3c2e7b775a8
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 11:52:37 2020 +0200

    test_client: Migrate multisteps_deposit scenario to use requests_mock_datadir
    
    Instead of mock.
    
    Note that right now, we should be using requests_mock_datadir_visits so we can
    leverage a second visit with the correct status. But this refused to work so
    far. A FIXME has been added to deal with this soon.

commit d3faa80e1dd1d97227895f012119fe37d044bdbe
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 19:32:30 2020 +0200

    test_client: Migrate slug generation scenario to use requests_mock_datadir
    
    instead of mock

commit 3917fed773647630ff78a3a2aa9414e316add909
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 19:22:46 2020 +0200

    test_client: Migrate metadata validation scenario to use requests_mock_datadir
    
    instead of mock

commit 3ee5ffd3962809cff6318f761b025d921824ba0a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 19:08:34 2020 +0200

    test_client: Migrate single deposit scenario to use requests_mock_datadir

commit 1dbcd76b8c0d3b4860c1ec36276a14916bce97e3
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 18:41:51 2020 +0200

    test_client: Migrate collection ok test scenario to use requests_mock_datadir
    
    instead of mock

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

Build is green

Patch application report for D4214 (id=14871)

Could not rebase; Attempt merge onto b9e98c0c96...

Updating b9e98c0c..24d3fd90
Fast-forward
 swh/deposit/tests/cli/test_client.py               | 275 +++++++++------------
 swh/deposit/tests/conftest.py                      |  14 ++
 .../tests/data/https_deposit.swh.test/1_test       |  19 ++
 .../https_deposit.test.metadata/1_servicedocument  |  26 ++
 .../tests/data/https_deposit.test.metadata/1_test  |  19 ++
 .../1_test_666_metadata                            |  19 ++
 .../https_deposit.test.metadata/1_test_666_status  |   8 +
 swh/deposit/tests/loader/conftest.py               |  14 --
 8 files changed, 218 insertions(+), 176 deletions(-)
 create mode 100644 swh/deposit/tests/data/https_deposit.swh.test/1_test
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_metadata
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_status
Changes applied before test
commit 24d3fd90a7d925f336cac9a836c5c4e1c60929c7
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

commit b18cd89d4c507709973721c47d49ffff7c856763
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 11:52:37 2020 +0200

    test_client: Migrate multisteps_deposit scenario to use requests_mock_datadir
    
    Instead of mock.
    
    Note that right now, we should be using requests_mock_datadir_visits so we can
    leverage a second visit with the correct status. But this refused to work so
    far. A FIXME has been added to deal with this soon.

commit 0d71b43ad3d86f3ca8065bb5e19cbc07e8abcfd4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 19:32:30 2020 +0200

    test_client: Migrate slug generation scenario to use requests_mock_datadir
    
    instead of mock

commit c377c3b66c3fcfc1093fd2f8f997ef487beebe7a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 19:22:46 2020 +0200

    test_client: Migrate metadata validation scenario to use requests_mock_datadir
    
    instead of mock

commit af8381f853f5144ee44a49bfcfb5efb5ef8646fa
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 19:08:34 2020 +0200

    test_client: Migrate single deposit scenario to use requests_mock_datadir

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

vlorentz added inline comments.
swh/deposit/tests/cli/test_client.py
228–235

I can't make sense of this variable name.

and why did you remove the length check of caplog.record_tuples?

swh/deposit/tests/cli/test_client.py
228–235

because there are more logs now (it's actually doing more queries now).
And those are not interesting for the tests.

Rename variable to something more clear (hopefully)

swh/deposit/tests/cli/test_client.py
228–235

it's actually doing more queries now

it's no longer preventing through mocks to query the service document, etc... (generating the xml file, etc...)
which can create more logs than the previous way of testing was doing.

i can't make sense...

I renamed the variable, hopefully, it's clearer now.

Build is green

Patch application report for D4214 (id=14893)

Could not rebase; Attempt merge onto af8381f853...

Updating af8381f8..404f2728
Fast-forward
 swh/deposit/tests/cli/test_client.py               | 270 +++++++++------------
 .../https_deposit.test.metadata/1_servicedocument  |  26 ++
 .../tests/data/https_deposit.test.metadata/1_test  |  19 ++
 .../1_test_666_metadata                            |  19 ++
 .../https_deposit.test.metadata/1_test_666_status  |   8 +
 5 files changed, 180 insertions(+), 162 deletions(-)
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_servicedocument
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_metadata
 create mode 100644 swh/deposit/tests/data/https_deposit.test.metadata/1_test_666_status
Changes applied before test
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

commit b18cd89d4c507709973721c47d49ffff7c856763
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Oct 9 11:52:37 2020 +0200

    test_client: Migrate multisteps_deposit scenario to use requests_mock_datadir
    
    Instead of mock.
    
    Note that right now, we should be using requests_mock_datadir_visits so we can
    leverage a second visit with the correct status. But this refused to work so
    far. A FIXME has been added to deal with this soon.

commit 0d71b43ad3d86f3ca8065bb5e19cbc07e8abcfd4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 19:32:30 2020 +0200

    test_client: Migrate slug generation scenario to use requests_mock_datadir
    
    instead of mock

commit c377c3b66c3fcfc1093fd2f8f997ef487beebe7a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Oct 8 19:22:46 2020 +0200

    test_client: Migrate metadata validation scenario to use requests_mock_datadir
    
    instead of mock

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

This revision is now accepted and ready to land.Oct 9 2020, 5:25 PM