Page MenuHomeSoftware Heritage

Factorize tests of the indexer storages.
ClosedPublic

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

Diff Detail

Repository
rDCIDX Metadata indexer
Branch
factorize-storage-tests2
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4061
Build 5340: tox-on-jenkinsJenkins
Build 5339: arc lint + arc unit

Event Timeline

  • Generalize test_revision_metadata_add_duplicate_twice.

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.

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.

swh/indexer/tests/storage/test_storage.py
339

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

cool

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.

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
This revision was automatically updated to reflect the committed changes.