Page MenuHomeSoftware Heritage

tests: Use in-memory storage and reflect changes from loader-core
ClosedPublic

Authored by anlambert on Dec 5 2018, 12:04 PM.

Details

Summary

Test for the subversion loader now use the in-memory storage.

Also use new test method names from swh-loader-core.

Related T1421

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Branch
tests-update
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2847
Build 3591: tox-on-jenkinsJenkins
Build 3590: arc lint + arc unit

Event Timeline

Update: Use better config value

Update: Revert temp_dir value to /tmp

ardumont added inline comments.
swh/loader/svn/tests/test_loader.py
28

What do you think of:

if not loader:
    loader = SvnLoaderMemoryStorage()
self.loader = loader

Sounds good.

I have some not blocking question/remark.

swh/loader/svn/tests/test_loader.py
104

Do we still do it that way?

I mean, IIRC, it was there to override the svn loader to bypass the real connection with the collaborator storage (no in-memory at the time).

Now that we are using the memory storage, we supposedly can set the collaborator memory storage up to do its job (here to not have any snapshot to return for the specified origin in that particular case).

But that can also be done later ;)

Note: I won't repeat this everywhere but that remark/question stands for all different loader implementations below ;)

This revision is now accepted and ready to land.Dec 5 2018, 12:46 PM
vlorentz requested changes to this revision.Dec 5 2018, 1:00 PM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/loader/svn/tests/test_loader.py
28
self.loader = loader or SvnLoaderMemoryStorage()

;)

104

Agreed, all classes with "Storage" in their name should be dropped

This revision now requires changes to proceed.Dec 5 2018, 1:00 PM
anlambert added inline comments.
swh/loader/svn/tests/test_loader.py
28

indeed, better

104

Will see what I can do

Update: Some refactoring

  • Use a single class for test loader: SvnLoaderTest, taking the latest snapshot revision to return as parameter
  • Slightly rename test classes

I like how that clarifies where to look for the snapshots ;)

Thanks.

swh/loader/svn/tests/test_loader.py
28

\m/

swh/loader/svn/tests/test_loader.py
230

rename in caps lock (that's a constant). Also probably split snapshot and revision.

236

Replace with:

super().setUp(loader=SvnLoaderTest)
self.storage.revision_add([_last_snp_rev['revision']])
self.storage.snapshot_add([_last_snp_rev['snapshot']])

And remove/rename all occurences of last_snp_rev.

268–287

kind of a duplicate of the global one.

anlambert added inline comments.
swh/loader/svn/tests/test_loader.py
230

ack

236

Unfortunately, that's not so simple. I have just tested to remove the swh_latest_snapshot_revision override in the test loader class in favor of only relying to the in-memory storage but numerous errors appeared. So I will keep it this way for the moment.

268–287

ack

This revision is now accepted and ready to land.Dec 5 2018, 2:48 PM

Update: Address vlorentz comments

You need to replace SWHLoader with BufferedLoader now

Update: final one after class remaming in loader-core

This revision was automatically updated to reflect the committed changes.