Page MenuHomeSoftware Heritage
Differential D6697

Add tests for conflict resolution functions
ClosedPublic
Actions

Authored by aeviso on Nov 26 2021, 5:08 PM.

Details

Reviewers
douardda
Group Reviewers
Reviewers
Commits
rDPROV6306b44896c1: Add tests for conflict resolution functions
Summary

Depends on D6339.

Diff Detail

Event Timeline

aeviso created this revision.Nov 26 2021, 5:08 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptNov 26 2021, 5:08 PM
swh-public-ci added a comment.Nov 26 2021, 5:14 PM

Build is green

Patch application report for D6697 (id=24331)

Could not rebase; Attempt merge onto 579c3bd35e...

Updating 579c3bd..40950ae
Fast-forward
 .gitignore                                       |   4 +-
 docs/storage/remote.rst                          | 720 +++++++++++++++++++++++
 mypy.ini                                         |   3 +
 pytest.ini                                       |   2 +
 requirements-test.txt                            |   1 +
 requirements.txt                                 |   1 +
 swh/provenance/__init__.py                       |   8 +
 swh/provenance/api/client.py                     | 463 +++++++++++++++
 swh/provenance/api/server.py                     | 646 +++++++++++++++++++-
 swh/provenance/cli.py                            |  31 +-
 swh/provenance/tests/conftest.py                 |  24 +-
 swh/provenance/tests/test_conflict_resolution.py | 143 +++++
 swh/provenance/util.py                           |   5 +
 tox.ini                                          |   3 +-
 14 files changed, 2040 insertions(+), 14 deletions(-)
 create mode 100644 docs/storage/remote.rst
 create mode 100644 swh/provenance/tests/test_conflict_resolution.py
Changes applied before test
commit 40950aedd630c874cfefdfeffe14fc3f56c0e786
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Tue Sep 28 10:01:43 2021 +0200

    Add tests for conflict resolution functions

commit 5f1d1b13504ee534f3726a8002d7d3a4609ac1c3
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Tue Sep 21 16:13:53 2021 +0200

    Add support for remote backend on existing storage tests

commit 84a62793d5de0b51aa7706b04a773366f5411391
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Fri Aug 20 12:21:27 2021 +0200

    Add new RabbitMQ-based client/server API
    
    Write methods in the `ProvenanceStorageInterface` are called through a server that
    guarantees conflict-free writings to the underlying database.
    
    Read methods are called directly from the client to avoid RCP overhead for reads.
    
    The server spawns multiple sub-processes to handle independent requests concurrently.

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

Harbormaster completed remote builds in B25205: Diff 24331.Nov 26 2021, 5:14 PM
aeviso requested review of this revision.Nov 26 2021, 5:14 PM
douardda accepted this revision.Nov 29 2021, 10:04 AM
douardda added a subscriber: douardda.

Fine for me; these tests could be written as parametrized tests (https://docs.pytest.org/en/6.2.x/parametrize.html#parametrize-basics) but not a big deal.

This revision is now accepted and ready to land.Nov 29 2021, 10:04 AM
aeviso added a comment.Nov 29 2021, 10:32 AM

When I did so with other tests your claim was that they were too difficult to follow and that tests should be as explicit as possible. These ones have a declarative function name for each case. What's the criteria after all?

douardda added a comment.Nov 29 2021, 12:08 PM
In D6697#174282, @aeviso wrote:

When I did so with other tests your claim was that they were too difficult to follow and that tests should be as explicit as possible. These ones have a declarative function name for each case. What's the criteria after all?

I don't know why you are talking about this. I am talking about something like this:

DATE = datetime.fromtimestamp(1000000000)
ONE_SEC = timedelta(seconds=1)
HASH = b"20329687bb9c1231a7e05afe86160343ad49b494"
RESOLVED_DATES = [
    (((HASH, ), (HASH, DATE), (HASH, None)), {HASH: DATE}),
    ((HASH, DATE), (HASH, DATE+ONE_SEC)), {HASH: DATE}),
]
@pytest.mark.parametrize("input,expected", RESOLVED_DATES) 
def test_resolve_dates() -> None:
    assert resolve_dates(input) == expected

I believe we can agree that this is not difficult to follow and read.

Also note that I accepted this diff, so I really don't understand what you are looking for with this...

aeviso updated this revision to Diff 24360.Nov 29 2021, 2:46 PM

rebase

aeviso added a child revision: D6705: Add test for routing key calculation.Nov 29 2021, 2:47 PM
aeviso added a comment.Nov 29 2021, 2:53 PM
DATE = datetime.fromtimestamp(1000000000)
ONE_SEC = timedelta(seconds=1)
HASH = b"20329687bb9c1231a7e05afe86160343ad49b494"
RESOLVED_DATES = [
    (((HASH, ), (HASH, DATE), (HASH, None)), {HASH: DATE}),
    ((HASH, DATE), (HASH, DATE+ONE_SEC)), {HASH: DATE}),
]
@pytest.mark.parametrize("input,expected", RESOLVED_DATES) 
def test_resolve_dates() -> None:
    assert resolve_dates(input) == expected

I believe we can agree that this is not difficult to follow and read.

I've never said this is difficult to read. Actually that's more in line to the previous test that I've originally submitted (for other methods) and I then I had to write in a more "explicit" on demand.

Also note that I accepted this diff, so I really don't understand what you are looking for with this...

What am I looking for? Simple, I'm trying to understand what the criteria are, to avoid having to rework the same stuff over and over again.
For now I'll stay with the more "explicit" form as that's what we've agreed before and I have several other test in the same style.

swh-public-ci added a comment.Nov 29 2021, 2:55 PM

Build is green

Patch application report for D6697 (id=24360)

Could not rebase; Attempt merge onto 579c3bd35e...

Updating 579c3bd..e4d0976
Fast-forward
 .gitignore                                       |   4 +-
 docs/storage/remote.rst                          | 720 +++++++++++++++++++++++
 mypy.ini                                         |   3 +
 pytest.ini                                       |   2 +
 requirements-test.txt                            |   1 +
 requirements.txt                                 |   1 +
 swh/provenance/__init__.py                       |   8 +
 swh/provenance/api/client.py                     | 466 +++++++++++++++
 swh/provenance/api/server.py                     | 666 ++++++++++++++++++++-
 swh/provenance/cli.py                            |  31 +-
 swh/provenance/tests/conftest.py                 |  24 +-
 swh/provenance/tests/test_conflict_resolution.py | 158 +++++
 swh/provenance/util.py                           |   5 +
 tox.ini                                          |   3 +-
 14 files changed, 2078 insertions(+), 14 deletions(-)
 create mode 100644 docs/storage/remote.rst
 create mode 100644 swh/provenance/tests/test_conflict_resolution.py
Changes applied before test
commit e4d097695294def8d809fbce6455b3c8712735dc
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Tue Sep 28 10:01:43 2021 +0200

    Add tests for conflict resolution functions

commit 4eecec5edb9139e945912ff39c7d0f412d3c9977
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Tue Sep 21 16:13:53 2021 +0200

    Add support for remote backend on existing storage tests

commit f8411a522e937b961d7fc8270c653d83039268fd
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Fri Aug 20 12:21:27 2021 +0200

    Add new RabbitMQ-based client/server API
    
    Write methods in the `ProvenanceStorageInterface` are called through a server that
    guarantees conflict-free writings to the underlying database.
    
    Read methods are called directly from the client to avoid RCP overhead for reads.
    
    The server spawns multiple sub-processes to handle independent requests concurrently.

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

Harbormaster completed remote builds in B25232: Diff 24360.Nov 29 2021, 2:55 PM
aeviso updated this revision to Diff 24368.Nov 29 2021, 4:01 PM

rebase

swh-public-ci added a comment.Nov 29 2021, 4:13 PM

Build is green

Patch application report for D6697 (id=24368)

Could not rebase; Attempt merge onto 579c3bd35e...

Updating 579c3bd..6306b44
Fast-forward
 .gitignore                                       |   4 +-
 docs/storage/remote.rst                          | 724 +++++++++++++++++++++++
 mypy.ini                                         |   3 +
 pytest.ini                                       |   2 +
 requirements-test.txt                            |   1 +
 requirements.txt                                 |   1 +
 swh/provenance/__init__.py                       |   8 +
 swh/provenance/api/client.py                     | 466 +++++++++++++++
 swh/provenance/api/server.py                     | 666 ++++++++++++++++++++-
 swh/provenance/cli.py                            |  31 +-
 swh/provenance/tests/conftest.py                 |  24 +-
 swh/provenance/tests/test_conflict_resolution.py | 158 +++++
 swh/provenance/util.py                           |   5 +
 tox.ini                                          |  38 +-
 14 files changed, 2117 insertions(+), 14 deletions(-)
 create mode 100644 docs/storage/remote.rst
 create mode 100644 swh/provenance/tests/test_conflict_resolution.py
Changes applied before test
commit 6306b44896c1775aef49b8840464efd80df9e648
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Tue Sep 28 10:01:43 2021 +0200

    Add tests for conflict resolution functions

commit e649205e258980cafbeb1db7d18f9c6efb1a8e76
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Tue Sep 21 16:13:53 2021 +0200

    Add support for remote backend on existing storage tests

commit a6cc3e4daf228ce9c124712b93c4749b16e65ce1
Author: Andres Ezequiel Viso <aeviso@softwareheritage.org>
Date:   Fri Aug 20 12:21:27 2021 +0200

    Add new RabbitMQ-based client/server API
    
    Write methods in the `ProvenanceStorageInterface` are called through a server that
    guarantees conflict-free writings to the underlying database.
    
    Read methods are called directly from the client to avoid RCP overhead for reads.
    
    The server spawns multiple sub-processes to handle independent requests concurrently.

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

Harbormaster completed remote builds in B25240: Diff 24368.Nov 29 2021, 4:13 PM
Closed by commit rDPROV6306b44896c1: Add tests for conflict resolution functions (authored by aeviso). · Explain WhyDec 1 2021, 3:50 PM
This revision was automatically updated to reflect the committed changes.
aeviso added a commit: rDPROV6306b44896c1: Add tests for conflict resolution functions.
aeviso removed a child revision: D6705: Add test for routing key calculation.Dec 1 2021, 3:52 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
provenance/
tests/
158 lines100%
Loading...

Diff 24360

View Options

swh/provenance/tests/test_conflict_resolution.py

Loading...
About · Developer information · Legal