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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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
17

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
78–79

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
17
self.loader = loader or SvnLoaderMemoryStorage()

;)

78–79

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
17

indeed, better

78–79

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
17

\m/

swh/loader/svn/tests/test_loader.py
145–169

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

175

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.

207–226

kind of a duplicate of the global one.

anlambert added inline comments.
swh/loader/svn/tests/test_loader.py
145–169

ack

175

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.

207–226

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.