Page MenuHomeSoftware Heritage

Add tests for swh.core.db.
ClosedPublic

Authored by vlorentz on Feb 11 2019, 3:43 PM.

Details

Summary

This increases coverage of the swh.core repository by:

  • initializing db tests
  • introducing hypothesis
  • refactoring a bit the connection initialization of the basedb module

Diff Detail

Repository
rDCORE Foundations and core functionalities
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont added inline comments.
requirements-test.txt
3

(>= 3.11.0)

Related P356

vlorentz added inline comments.
requirements-test.txt
3

Even without regexps?

requirements-test.txt
3

well, it works!

I added it to the scheduler (matching the one we already have on indexer) and nothing complained.

[1] is proof as it built (we can see debian tags and everything)

[1] https://forge.softwareheritage.org/rDSCH3488c265f4e3f70c8591dd1c7064388b312e9144

3

no parenthesis though, don't know why i put that there.

anlambert added a subscriber: anlambert.

Looks good to me!

swh/core/tests/test_db.py
60

Just a nitpick, I would put the strategy implementation into a dedicated function and named it according to your desired test inputs instead.
I think tests are more readable this way.

This revision is now accepted and ready to land.Feb 11 2019, 4:12 PM
swh/core/db/__init__.py
111

I'm unsure of that factorisation (because i'm not so sure of the workflow around those calls: from_pool, connect).
What i'm sure though is that it could be cause of regression.
I'd feel safer if we added tests around those cases.

Reading swh.storage.{storage,db} and scheduler's, the initialization sounds fine with this change but still...

Not sure about the initialization refactoring and its impacts on the other modules.

Other than that, the rest sounds quite fine, thanks.

vlorentz added inline comments.
swh/core/db/__init__.py
111

That will only be an issue when a connection object is created directly, instead of using one of these classmethods. grep "psycopg2\.conn" ../*/swh/ -r shows only two results outside swh.core: the vault (before D1108) and the swh.journal.checker. Neither use BaseDb.

I'd feel safer if we added tests around those cases.

Which ones?

Which ones?

the @classmethod you changed from_pool and connect?

  • Rebase
  • Move the hypothesis given outside the class.
  • Add test for BaseDb.connect.
This revision was automatically updated to reflect the committed changes.