Skip to content
Snippets Groups Projects

Add minimal test coverage for swh.storage.schemata

2 unresolved threads

Adds a sqlalchemy test fixture on top of pytest_postgresql, as well as a regression test for !289 (closed).

Depends on !289 (closed).

Test Plan

tox


Migrated from D2264 (view on Phabricator)

Merge request reports

Approved by

Closed by Phabricator Migration userPhabricator Migration user 5 years ago (Nov 13, 2019 12:13pm UTC)

Merge details

  • The changes were not merged into generated-differential-D2264-target.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
1 # Copyright (C) 2019 The Software Heritage developers
2 # See the AUTHORS file at the top-level directory of this distribution
3 # License: GNU General Public License version 3, or any later version
4 # See top-level LICENSE file for more information
5
6 import pytest
7 from pytest_postgresql.janitor import DatabaseJanitor
8 from sqlalchemy import create_engine
9
10
11 @pytest.fixture
  • 1 # Copyright (C) 2019 The Software Heritage developers
    2 # See the AUTHORS file at the top-level directory of this distribution
    3 # License: GNU General Public License version 3, or any later version
    4 # See top-level LICENSE file for more information
    5
    6 import pytest
    7 from swh.storage.schemata.distribution import SQLBase, Distribution, Area
    8
    9 from sqlalchemy.orm import sessionmaker
    10
    11
    12 @pytest.fixture
  • pytest fixtures declaration should be simplified.

    Otherwise, this feels weird to have database declarations only related to the debian lister/loader in swh-storage. Maybe this could be moved in swh-lister or swh-loader-debian ?

  • Merge request was returned for changes

  • Author Maintainer

    ! In !290 (closed), @anlambert wrote: Otherwise, this feels weird to have database declarations only related to the debian lister/loader in swh-storage. Maybe this could be moved in swh-lister or swh-loader-debian ?

    Both the lister and loader use these models; It made sense, at the time it was introduced, to put this functionality in a swh.storage subpackage, rather than introduce a dependency between lister and loader.

  • Author Maintainer

    Use @pytest.fixture instead of yield_fixture

  • mentioned in merge request swh-core!98 (closed)

  • mentioned in merge request swh-lister!387 (closed)

  • Merge request was accepted

  • Antoine Lambert approved this merge request

    approved this merge request

  • Author Maintainer

    Merge request was merged

  • Otherwise, this feels weird to have database declarations only related to the debian lister/loader in swh-storage. Maybe this could be moved in swh-lister or swh-loader-debian ?

    For info, it's already been moved to swh-lister (see swh.lister.debian.models which is almost the same as schemata, almost because it got slightly adapted since). As i am currently deploying the latest lister-debian and the new debian-loader... I waited for this to be deployed and running in production prior to remove it from swh.storage (the old debian-loader still depends on this even if it's not really running). I'll do migrate those tests as well in due time.

    Cheers,

  • Otherwise, this feels weird to have database declarations only related to the debian lister/loader in swh-storage. Maybe this could be moved in swh-lister or swh-loader-debian ?

    For info, it's already been moved to swh-lister (see swh.lister.debian.models which is almost the same as schemata, almost because it got slightly adapted since). As i am currently deploying the latest lister-debian and the new debian-loader... I waited for this to be deployed and running in production prior to remove it from swh.storage (the old debian-loader still depends on this even if it's not really running). I'll do migrate those tests as well in due time.

    done in swh-lister!387 (closed) btw

  • Please register or sign in to reply
    Loading