Page MenuHomeSoftware Heritage

tests: Refactor scenario about visits on repository with no change
ClosedPublic

Authored by ardumont on Jul 3 2020, 7:33 AM.

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

ardumont created this revision.Jul 3 2020, 7:33 AM

Build is green

Patch application report for D3399 (id=12050)

Could not rebase; Attempt merge onto ee23fd7582...

Updating ee23fd7..a846f61
Fast-forward
 swh/loader/svn/tests/conftest.py    |   9 ++-
 swh/loader/svn/tests/test_loader.py | 140 +++++++++++++++++++-----------------
 2 files changed, 80 insertions(+), 69 deletions(-)
Changes applied before test
commit a846f61d164da4ee826ae2fcc48f965b307e6f67
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 3 07:29:35 2020 +0200

    tests: Refactor scenario about visits on repository with no change
    
    Related to T2462

commit 5d770ec3934b84413b13b63a19443592832c8e0a
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu Jul 2 19:19:13 2020 +0200

    Start tests with pytest refactoring: Add scenario for 1 visit
    
    This starts to align the existing svn tests with the core/package ones. For
    now, utilities functions are in this repository close to the tests. They will
    most possibly be refactored out in swh-loader-core at some point.
    
    Related to T2462

See https://jenkins.softwareheritage.org/job/DLDSVN/job/tests-on-diff/33/ for more details.

vlorentz added inline comments.
swh/loader/svn/tests/test_loader.py
246

why is this eventful if it loaded exactly the same thing?

265–274

what happened to this?

ardumont added inline comments.Jul 3 2020, 9:52 AM
swh/loader/svn/tests/test_loader.py
246

That's a "bug" in the current implementation.
Which this refactoring is revealing ;)

ardumont added inline comments.Jul 3 2020, 9:54 AM
swh/loader/svn/tests/test_loader.py
265–274

That's the point of this refactoring.
I'm removing it in favor of the pytest way of testing it (like the other loaders).

The BaseSvnLoaderTest scaffolding will disappear.

vlorentz added inline comments.Jul 3 2020, 10:05 AM
swh/loader/svn/tests/test_loader.py
265–274

I understand, but you didn't add an equivalent of these assertions, so the new tests aren't checking this

vlorentz accepted this revision.Jul 3 2020, 10:06 AM
vlorentz added inline comments.
swh/loader/svn/tests/test_loader.py
246

Could you add a FIXME/TODO as a comment, then?

This revision is now accepted and ready to land.Jul 3 2020, 10:06 AM
ardumont added inline comments.Jul 3 2020, 10:10 AM
swh/loader/svn/tests/test_loader.py
246

Yeah, i guess i can.

To be clear, i intended to attend to this after the refactoring.
Same about the simplification on this loader's initialization which is way too complicated.
(it's pre-snapshot).

265–274

Ah there is kinda something about this, with the get_stats part.
But i'm only checking the origin-visit/snapshot changes.

The thing is, i'm not sure we can do what's in that test. I think it was
wrongly possible due to the scaffolding which i want to remove.

See, loading twice the same repository with no changes, we should have an
"uneventful" scenario but we don't.

ardumont updated this revision to Diff 12064.Jul 3 2020, 10:45 AM

Add fixme around the loader's "uneventful" behavior inconsistency

(As this diff is about refactoring the tests, it does not touch the loader heh,
so adding the fixme is the sole reasonable approach right now)

ardumont updated this revision to Diff 12065.Jul 3 2020, 10:46 AM

Amend with correct commit

Build is green

Patch application report for D3399 (id=12064)

Rebasing onto 5c46affea6...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-46-D3399.
Changes applied before test

See https://jenkins.softwareheritage.org/job/DLDSVN/job/tests-on-diff/46/ for more details.

Build is green

Patch application report for D3399 (id=12065)

Rebasing onto 5c46affea6...

Current branch diff-target is up to date.
Changes applied before test
commit 58bb1e4685046485b090bddb82fcb3935746631d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Fri Jul 3 07:29:35 2020 +0200

    tests: Refactor scenario about visits on repository with no change
    
    Related to T2462

See https://jenkins.softwareheritage.org/job/DLDSVN/job/tests-on-diff/47/ for more details.