Page MenuHomeSoftware Heritage

Start writing tests for the deposit upload CLI.
ClosedPublic

Authored by vlorentz on Dec 17 2019, 6:53 PM.

Details

Diff Detail

Repository
rDDEP Push deposit
Branch
test-cli-deposit-upload
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9850
Build 14537: tox-on-jenkinsJenkins
Build 14536: arc lint + arc unit

Event Timeline

Thanks for this ;)

I'll have a closer look tomorrow.

At first glance, i think you mixed multiple diffs' content with this one though (i see sentry sdk code and the atom stuff moved around which are in other accepted diffs)
;)

ardumont added a subscriber: moranegg.

I have some non-blocking suggestions.

And thanks again!

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

why don't you make that a fixture?

@pytest.fixture
def cli_client(mocker):
    mock_client = MagicMock()
    mock_client.service_document.return_value = EXAMPLE_SERVICE_DOCUMENT
    mock_client.deposit_create.return_value = '{"foo": "bar"}'
    mocker.patch(
        'swh.deposit.cli.client._client',
        return_value=mock_client)
    return mock_client

That will unclutter a bit the scenario below.
If that makes sense.

Note:
you can use the factory pattern if you need some different output in the deposit.create.return_value mock.

85
99

jsyk, as @moranegg probably already told you this is kind of a pain in deposit.

The slug is a word used in the sword v2 protocol (it makes no sense to me).
It's needed because it's required in the spec.
Thus, the uuid generation somewhere in the cli here when it's not provided (it needs to be unique).

We decided to use this as external_id, so whenever you see that, think external_id as a unique identifier for the client system...

I don't like the fact that we use both terms but i was a bit worried to unify it under what term and hide that somewhere at the time. Maybe it'd make more sense to rename it once and for all though (later, not right now ;).

This revision is now accepted and ready to land.Dec 19 2019, 8:52 AM

I find this nicer with the deduplication, thanks.

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

s/what term/one term/