Page MenuHomeSoftware Heritage
Differential D682

Use the in-mem storage instead of mocks in BaseLoaderTest.
ClosedPublic
Actions

Authored by vlorentz on Nov 20 2018, 3:56 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T1307: Remove mock storages used in tests.
Commits
rDLDBASE9a0b8eb67516: Use the in-mem storage instead of mocks in BaseLoaderTest.
Summary

This Diff breaks all existing loader tests.

Diff Detail

Repository
rDLDBASE Generic VCS/Package Loader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Nov 20 2018, 3:56 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptNov 20 2018, 3:56 PM
vlorentz added a child revision: D684: Use the in-mem storage instead of mocks to test the git loader..Nov 20 2018, 3:59 PM
swh-public-ci added a subscriber: swh-public-ci.Nov 20 2018, 4:10 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/18/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/18/console

Harbormaster failed remote builds in B2464: Diff 2144!Nov 20 2018, 4:10 PM
vlorentz added a comment.Nov 20 2018, 4:15 PM

The build is failing because this change depends on an unreleased version of swh.storage.

vlorentz added a task: T1307: Remove mock storages used in tests..Nov 20 2018, 4:17 PM
vlorentz retitled this revision from Use the in-mem storage instead of mocks. to Use the in-mem storage instead of mocks in BaseLoaderTest..
ardumont accepted this revision.Nov 20 2018, 5:03 PM
ardumont added a subscriber: ardumont.
ardumont added inline comments.
swh/loader/core/tests/__init__.py
174

self.assertIsNotNone(snap)?

286

\m/

This revision is now accepted and ready to land.Nov 20 2018, 5:03 PM
ardumont added a comment.EditedNov 21 2018, 8:59 AM

The build is failing because this change depends on an unreleased version of swh.storage.

I've just released one, meaning:

  • tag repository
  • build debian package (for consistency, 1 tag, 1 debian package so far)
  • go to jenkins, trigger back the build on the latest master. Wait for it to complete. That picks up the publish step to pypi (it did ;)

So we should be able to trigger the build back now and this should be fine.

ardumont added a comment.Nov 21 2018, 9:17 AM

So we should be able to trigger the build back now and this should be fine.

Unfortuntately, not yet. Everything i said is ok.
But the storage got pushed to test.pypi.org... and not pypi.org...

ardumont added a comment.Nov 21 2018, 10:14 AM

But the storage got pushed to test.pypi.org... and not pypi.org...

Something was wrong in the deployment. douardda fixed it and now the new version got uploaded to pypi.
I restarted the build to check if everything is fine here.

swh-public-ci added a comment.Nov 21 2018, 10:15 AM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/19/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/19/console

ardumont requested changes to this revision.EditedNov 21 2018, 10:24 AM

I restarted the build to check if everything is fine here.
BUILD has failed.

It's not fine.
There are real errors.
You did not check the test_loader.py module.

This revision now requires changes to proceed.Nov 21 2018, 10:24 AM
vlorentz updated this revision to Diff 2152.Nov 21 2018, 3:27 PM
  • Adapt test_loader.py to work without LoaderNoStorage. I removed most of the classes because it's pointless to have so much code just to test duplicated threeliner functions.
vlorentz updated this revision to Diff 2153.Nov 21 2018, 3:28 PM
  • assertIsNotNone
vlorentz added a parent revision: D680: Make it official that content_missing accepts missing keys..Nov 21 2018, 3:28 PM
In D682#14187, @ardumont wrote:

I restarted the build to check if everything is fine here.
BUILD has failed.

It's not.
There are real errors.
You did not check the test_loader.py module.

Oops.

Now it actually fails because it depends on another diff ( D680) ^^

swh-public-ci added a comment.Nov 21 2018, 3:29 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/20/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/20/console

Harbormaster failed remote builds in B2490: Diff 2152!Nov 21 2018, 3:29 PM
swh-public-ci added a comment.Nov 21 2018, 3:30 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/21/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/21/console

Harbormaster failed remote builds in B2491: Diff 2153!Nov 21 2018, 3:30 PM
ardumont added a comment.Nov 21 2018, 3:40 PM

I removed most of the classes because it's pointless to have so much code just to test duplicated threeliner functions.

I disagree. It's not pointless. Well, it was not prior to this diff.
The point was to have enough coverage for others to be able to touch those with some parachute.

Now that you developed the real in-memory storage, those tests are no longer relevant.
So now, it's ok for those to go away. The in-memory storage is tested well enough ;)

ardumont accepted this revision.Nov 21 2018, 3:58 PM

Now it actually fails because it depends on another diff ( D680) ^^

I'll trust you on this ;)

This revision is now accepted and ready to land.Nov 21 2018, 3:58 PM
vlorentz added a comment.Nov 21 2018, 4:03 PM
In D682#14298, @ardumont wrote:

Now it actually fails because it depends on another diff ( D680) ^^

I'll trust you on this ;)

It will make all tests fail from now on, I would rather not land it

vlorentz updated this revision to Diff 2207.Nov 22 2018, 4:54 PM
  • Don't initialize the storage, use the loader's instead.
swh-public-ci added a comment.Nov 22 2018, 4:56 PM

Build has FAILED

Link to build: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/23/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDBASE/job/tox/23/console

Harbormaster failed remote builds in B2543: Diff 2207!Nov 22 2018, 4:56 PM
vlorentz added a comment.Nov 27 2018, 6:32 PM

Diffs are piling on top of this one, so I'll land it anyway.

Closed by commit rDLDBASE9a0b8eb67516: Use the in-mem storage instead of mocks in BaseLoaderTest. (authored by vlorentz). · Explain WhyNov 27 2018, 6:33 PM
This revision was automatically updated to reflect the committed changes.
vlorentz mentioned this in D770: (Re-)add tests for the loader..Dec 4 2018, 3:47 PM
vlorentz mentioned this in rDLDBASE750a1877a2f6: (Re-)add tests for the loader..Dec 5 2018, 10:06 AM

Revision Contents
Changeset List

PathSize
swh/
loader/
core/
tests/
138 lines
342 lines

Diff 2271

View Options

swh/loader/core/tests/__init__.py

Loading...
View Options

swh/loader/core/tests/test_loader.py

Loading...
About · Developer information · Legal