Differential D1095

Factorize tests of the indexer storages.
ClosedPublic
Actions

Authored by vlorentz on Feb 7 2019, 2:05 PM.

Details

Reviewers

Group Reviewers

Commits

rDCIDXa4893e6caa1c: Factorize tests of the indexer storages.

Diff Detail

Repository

rDCIDX Metadata indexer

Branch

factorize-storage-tests2

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 4071
Build 5358: tox-on-jenkins	Jenkins
Build 5357: arc lint + arc unit

Event Timeline

vlorentz created this revision.Feb 7 2019, 2:05 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 7 2019, 2:05 PM

Harbormaster failed remote builds in B4058: Diff 3431!Feb 7 2019, 2:05 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/355/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/355/console

vlorentz added a parent revision: D1079: Add checks in the idx_storage that the same content/rev/orig is not present twice in the new data..Feb 7 2019, 2:06 PM

Generalize test_revision_metadata_add_duplicate_twice.

Harbormaster failed remote builds in B4059: Diff 3432!Feb 7 2019, 2:13 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/356/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/356/console

vlorentz mentioned this in D1079: Add checks in the idx_storage that the same content/rev/orig is not present twice in the new data..Feb 7 2019, 2:25 PM

I'm not sure we gain here. It obfuscates the tests.

well, we do avoid duplication in test data but then the test data is sparse.
In the setup of the tests, we have a partial view of the overall expected results.

futhermore, it seems we lose some tests (e.g. ctags) for example.

I'm mitigated over this one. Another point of view would be nice
@douardda @anlambert any thoughts?

swh/indexer/tests/storage/test_storage.py
339	It seems we lose coverage here. Do not migrate those tests to the new way and avoid losing coverage.

In D1095#23164, @ardumont wrote:

I'm not sure we gain here. It obfuscates the tests.

On the contrary, I find that factorizing them makes them easier to read, as I don't have to read five times almost-the-same-but-not-really code

well, we do avoid duplication in test data but then the test data is sparse.

We also avoid duplication of the test case (which we copy-pasted several times), sometimes with mistakes; that's why this factoring caught a bug.

futhermore, it seems we lose some tests (e.g. ctags) for example.

No we don't. I preserved tests that I could not factorize easily.

ardumont added inline comments.Feb 7 2019, 2:34 PM

swh/indexer/tests/storage/test_storage.py
339	vlorentz: No we don't. I preserved tests that I could not factorize easily. cool

vlorentz marked 2 inline comments as done.Feb 7 2019, 2:36 PM

good points.

On the contrary, I find that factorizing them makes them easier to read, as I don't have to read five times almost-the-same-but-not-really code

Yes and no

Yes, the gist of it is.

No, because the way it's plugged together is not so clear (@rename decorator, tests generation)...
This is no longer simple tests (one might ask where are the tests for the tests plumbing? ;)

i won't block this though. Let's see where this goes.

Rewrite as a parent of D1079

vlorentz removed a parent revision: D1079: Add checks in the idx_storage that the same content/rev/orig is not present twice in the new data..Feb 7 2019, 3:00 PM

vlorentz added a child revision: D1079: Add checks in the idx_storage that the same content/rev/orig is not present twice in the new data..

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/358/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/358/console

Harbormaster failed remote builds in B4061: Diff 3434!Feb 7 2019, 3:02 PM

Rebase on top of D1096

vlorentz added a parent revision: D1096: Update mapping column when updating existing metadata..Feb 7 2019, 3:09 PM

Harbormaster failed remote builds in B4064: Diff 3437!Feb 7 2019, 3:10 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/361/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/361/console

Fix Diff reordering.

Harbormaster failed remote builds in B4066: Diff 3438!Feb 7 2019, 3:10 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/362/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/362/console

ardumont edited the summary of this revision. (Show Details)Feb 7 2019, 3:17 PM

rebase

Build is green
See https://jenkins.softwareheritage.org/job/DCIDX/job/tox/366/ for more details.

Harbormaster completed remote builds in B4071: Diff 3443.Feb 7 2019, 3:24 PM

i won't block this though. Let's see where this goes.

here it goes.

This revision is now accepted and ready to land.Feb 7 2019, 3:24 PM

rebase

Closed by commit rDCIDXa4893e6caa1c: Factorize tests of the indexer storages. (authored by vlorentz). · Explain WhyFeb 7 2019, 3:28 PM

This revision was automatically updated to reflect the committed changes.

Harbormaster failed remote builds in B4080: Diff 3452!Feb 7 2019, 3:28 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/371/
See console output for more information: https://jenkins.softwareheritage.org/job/DCIDX/job/tox/371/console

Revision Contents
Changeset List

Path

Size

swh/

indexer/

tests/

storage/

test_storage.py

1018 lines

Diff 3443

swh/indexer/tests/storage/test_storage.py

Loading...