Page MenuHomeSoftware Heritage

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

Authored by anlambert on Wed, Dec 5, 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

anlambert created this revision.Wed, Dec 5, 12:04 PM
anlambert updated this revision to Diff 2441.Wed, Dec 5, 12:09 PM

Update: Fix config value

anlambert updated this revision to Diff 2443.Wed, Dec 5, 12:10 PM

Update: Use better config value

anlambert updated this revision to Diff 2445.Wed, Dec 5, 12:32 PM

Update: Revert temp_dir value to /tmp

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

What do you think of:

if not loader:
    loader = SvnLoaderMemoryStorage()
self.loader = loader
ardumont accepted this revision.Wed, Dec 5, 12:46 PM

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.Wed, Dec 5, 12:46 PM
vlorentz requested changes to this revision.Wed, Dec 5, 1:00 PM
vlorentz added a subscriber: vlorentz.
vlorentz added inline comments.
swh/loader/svn/tests/test_loader.py
18
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.Wed, Dec 5, 1:00 PM
anlambert marked 2 inline comments as done.Wed, Dec 5, 1:12 PM
anlambert added inline comments.
swh/loader/svn/tests/test_loader.py
18

indeed, better

78–79

Will see what I can do

anlambert updated this revision to Diff 2450.Wed, Dec 5, 1:52 PM

Update: Some refactoring

  • Use a single class for test loader: SvnLoaderTest, taking the latest snapshot revision to return as parameter
  • Slightly rename test classes
ardumont accepted this revision.Wed, Dec 5, 2:06 PM

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

Thanks.

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

\m/

vlorentz added inline comments.Wed, Dec 5, 2:08 PM
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.

208–227

kind of a duplicate of the global one.

anlambert marked 3 inline comments as done.Wed, Dec 5, 2:48 PM
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.

208–227

ack

vlorentz accepted this revision.Wed, Dec 5, 2:48 PM
This revision is now accepted and ready to land.Wed, Dec 5, 2:48 PM
anlambert updated this revision to Diff 2461.Wed, Dec 5, 2:48 PM

Update: Address vlorentz comments

You need to replace SWHLoader with BufferedLoader now

anlambert updated this revision to Diff 2464.Wed, Dec 5, 2:58 PM

Update: final one after class remaming in loader-core

This revision was automatically updated to reflect the committed changes.