Page MenuHomeSoftware Heritage

swh.loader.base: Add tests
AbandonedPublic

Authored by ardumont on Jul 18 2019, 9:40 AM.

Details

Reviewers
nahimilega
Group Reviewers
Reviewers
Summary

Add tests for base loader
Depends on D1814

Diff Detail

Repository
rDLDBASE Generic VCS/Package Loader
Branch
test
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 7140
Build 10070: tox-on-jenkinsJenkins
Build 10069: arc lint + arc unit

Event Timeline

nahimilega created this revision.Jul 18 2019, 9:40 AM
nahimilega planned changes to this revision.Jul 19 2019, 10:06 AM
nahimilega updated this revision to Diff 5976.Jul 24 2019, 8:29 PM
  • swh.loader.base: Add base tests for download and loader
nahimilega added inline comments.Jul 25 2019, 9:58 AM
swh/loader/base/tests/base_test_loader.py
2 ↗(On Diff #5976)

This name of the file is weird and I suppose we need to change it, but it can't have word "test" in the start or end of the name because then it would be detected as a separate test which is not true. it is meant to work when overridden in the loader(see GNU loader for example).
Any name suggestions?

swh/loader/base/tests/download_test_base.py
5 ↗(On Diff #5976)

Are these tests enough, do we need more tests?
I tried to cover every possible scenario which is not specific to any loader. Tests specific to some loader could be written separately.

(same question for the file above this base__test_loader.py)

ardumont added inline comments.Jul 25 2019, 1:41 PM
swh/loader/base/tests/base_test_loader.py
2 ↗(On Diff #5976)

This name of the file is weird ...
Any name suggestions?

No better suggestion, i'd say that will do for now.

A remark though, we are trying (again, that takes time to converge) to move away from tests defined within classes.
I know you don't really have the examples.
Because the code you start from was written prior to those decisions, but still, can you please have a look at pytest's documentation (pytest fixture) to work towards those?

Thanks

A remark though, we are trying (again, that takes time to converge) to move away from tests defined within classes.
I know you don't really have the examples.
Because the code you start from was written prior to those decisions, but still, can you please have a look at pytest's documentation (pytest fixture) to work towards those?

Referencing the task helps.
Related T1261

Note:
I did not ask earlier for the listers because that was not a bootstrap phase.
You added new listers and it was reasonable to start using the existing tests base code.
Here, i think it's reasonable to start testing with this in mind.

Thanks in advance.

nahimilega added a comment.EditedJul 25 2019, 1:51 PM

I will look at pytest documentation and try to convert these tests according to the new format. One doubt though.

to move away from tests defined within classes.

If these all tests are not concealed in a class then how will these tests be inherited by all the loader?

One possible solution I can think of is importing function instead of class and then calling those functions from the all loader tests.

nahimilega updated this revision to Diff 6103.Aug 4 2019, 11:26 PM
  • convert tests in pytest format

Please either abandon the diff or mark this as planned changes like the others ;)

nahimilega planned changes to this revision.Aug 6 2019, 2:44 PM
ardumont commandeered this revision.Oct 1 2019, 1:26 PM
ardumont abandoned this revision.
ardumont added a reviewer: nahimilega.