No notice given when db migrations are not run due to missing engine

Bug #1327397 reported by devananda on 2014-06-06
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Ironic
Fix Released
Low
devananda
OpenStack Compute (nova)
Low
Unassigned
oslo-incubator
High
Ilya Pekelny

Bug Description

When the unit test suite is run and there is no backend configured for testing migrations, the test passes rather than indicating that it was skipped. This has caused developers to assume the test passed when, in fact, it never ran.

To run the migration tests, one must configure a storage backend in tests/db/sqlalchemy/test_migrations.conf

In the absence of this, TestMigrations.test_walk_versions should be skipped.

Tags: db Edit Tag help
devananda (devananda) on 2014-06-06
Changed in ironic:
status: New → Triaged
importance: Undecided → Low

Fix proposed to branch: master
Review: https://review.openstack.org/98504

Changed in ironic:
assignee: nobody → Devananda van der Veen (devananda)
status: Triaged → In Progress
tags: added: db
Changed in oslo:
importance: Undecided → High

Reviewed: https://review.openstack.org/98504
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=f9c7e4344a49a37edbb575e0b37254a8714d1d38
Submitter: Jenkins
Branch: master

commit f9c7e4344a49a37edbb575e0b37254a8714d1d38
Author: Devananda van der Veen <email address hidden>
Date: Fri Jun 6 12:42:00 2014 -0700

    Skip migrations test_walk_versions instead of pass

    When there is no db engine configured for the migration tests,
    the test_walk_versions test was passing, rather than skipping,
    because it contained a for-loop and no check for the length of the
    sequence being iterated over.

    This patch checks the length of the configured engines and
    properly skips the test when there are no engines.

    Change-Id: I8a5c0a66f65a99c5169b251be02a8fbe6c727641
    Closes-bug: #1327397

Changed in ironic:
status: In Progress → Fix Committed
Thierry Carrez (ttx) on 2014-06-12
Changed in ironic:
milestone: none → juno-1
status: Fix Committed → Fix Released
Joe Gordon (jogo) on 2014-06-13
Changed in nova:
status: New → Confirmed
importance: Undecided → Low

Reviewed: https://review.openstack.org/100024
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=08201764d1500a4bff5db91228659a2c4063b808
Submitter: Jenkins
Branch: master

commit 08201764d1500a4bff5db91228659a2c4063b808
Author: Joe Gordon <email address hidden>
Date: Fri Jun 13 14:46:37 2014 -0700

    Skip migrations test_walk_versions instead of pass

    When there is no db engine configured for the migration tests,
    the test_walk_versions test was passing, rather than skipping,
    because it contained a for-loop and no check for the length of the
    sequence being iterated over.
    This patch checks the length of the configured engines and
    properly skips the test when there are no engines.

    Change-Id: I4078655f01638c22cf029f088018b8be93d6a01a
    Co-Authored-By: Devananda van der Veen<email address hidden>
    Closes-bug: #1327397

Changed in nova:
status: Confirmed → Fix Committed
Ilya Pekelny (i159) on 2014-06-23
Changed in oslo:
assignee: nobody → Ilya Pekelny (i159)
status: New → In Progress
haruka tanizawa (h-tanizawa) wrote :

I have no permission to change nova's assignee:
Unassigned -> Joe Gordon
Thanks.

Reviewed: https://review.openstack.org/93424
Committed: https://git.openstack.org/cgit/openstack/oslo.db/commit/?id=c34c32e09ed1f170dfe62c7daedf0cd60f57a833
Submitter: Jenkins
Branch: master

commit c34c32e09ed1f170dfe62c7daedf0cd60f57a833
Author: Ilya Pekelny <email address hidden>
Date: Tue May 13 14:44:54 2014 +0300

    Opportunistic migration tests

    Migrations should be tested with real database backends. For this goal
    intended number of base test cases which used in current implementation.
    Previously there were two ways to run migration tests: using
    opportunistic test cases (default way we've been using on CI) and
    by using database connection strings, specified in test_migrations.conf,
    for every particular database test case. For the sake of simplicity and
    consistency we are moving to using of opportunistic db test cases here.

    With this change we are free from locking, so we don't need `lockfile`
    anymore.

    Closes-Bug: #1327397
    Closes-Bug: #1328997
    Change-Id: I92b1dcd830c4755f429a0f6529911e607c2c7de7

Changed in oslo:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/107053
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=ffd83dc3af170608a867ed1721a90747e67d2934
Submitter: Jenkins
Branch: master

commit ffd83dc3af170608a867ed1721a90747e67d2934
Author: Victor Sergeyev <email address hidden>
Date: Tue Jul 15 12:41:34 2014 +0300

    Use opportunistic approach for migration testing

    Refactored migration tests due to use OpportunisticTestCase, removed
    unused code and ``test_migrations.conf`` file.

    This change allows tests use database ``openstack_citest`` only
    for connection to the database backend. The main feature of this
    approach is - for each migration test will be created new database
    with random name. This will avoid migration tests of race conditions
    and reduce tests intersection.

    ``test_migrations.conf`` file was removed, because we create test
    database for migration test, so we no longer need test database credentials.

    Closes-Bug: #1327397
    Closes-Bug: #1328997

    Change-Id: I95ad140ba5f483cd3dc36e2b78f140826d57624f

Changed in nova:
milestone: none → juno-2
status: Fix Committed → Fix Released
Changed in oslo:
milestone: none → juno-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2014-10-16
Changed in nova:
milestone: juno-2 → 2014.2
Thierry Carrez (ttx) on 2014-10-16
Changed in ironic:
milestone: juno-1 → 2014.2
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers