← Back to team overview

yahoo-eng-team team mailing list archive

[Bug 1431571] [NEW] ArchiveTestCase erroneously assumes the tables that are populated

 

Public bug reported:

Running subsets of Nova tests or individual tests within test_db_api
reveals a simple error in several of the tests within ArchiveTestCase.

A test such as test_archive_deleted_rows_2_tables attempts the
following:

1. places six rows into instance_id_mappings
2. places six rows into instances
3. runs the archive_deleted_rows_ routine with a max of 7 rows to archive
4. runs a SELECT of instances and instance_id_mappings, and confirms that only 5 remain.

Running this test directly with PYTHONHASHSEED=random will very easily
encounter failures such as:

Traceback (most recent call last):
  File "/Users/classic/dev/redhat/openstack/nova/nova/tests/unit/db/test_db_api.py", line 7869, in test_archive_deleted_rows_2_tables
    self.assertEqual(len(iim_rows) + len(i_rows), 5)
  File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 350, in assertEqual
    self.assertThat(observed, matcher, message)
  File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 435, in assertThat
    raise mismatch_error
testtools.matchers._impl.MismatchError: 8 != 5


or 

Traceback (most recent call last):
  File "/Users/classic/dev/redhat/openstack/nova/nova/tests/unit/db/test_db_api.py", line 7872, in test_archive_deleted_rows_2_tables
    self.assertEqual(len(iim_rows) + len(i_rows), 5)
  File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 350, in assertEqual
    self.assertThat(observed, matcher, message)
  File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 435, in assertThat
    raise mismatch_error
testtools.matchers._impl.MismatchError: 10 != 5


The reason is that the archive_deleted_rows() routine looks for rows in *all* tables, in *non-deterministic order*, e.g. by searching through "models.__dict__.itervalues()".   In the "8 != 5" case, there are rows present also in the instance_types table.  By PDBing into archive_deleted_rows during the test, we can see here:

ARCHIVED 4 ROWS FROM TABLE instances
ARCHIVED 3 ROWS FROM TABLE instance_types
Traceback (most recent call last):
...
testtools.matchers._impl.MismatchError: 8 != 5

that is, the archiver locates seven rows just between instances and
instance_types, then stops.  It never even gets to the
instance_id_mappings table.

The serious problem with the way this test is designed, is that if we
were to make it ignore only certain tables, or make the ordering fixed,
or anything else, that will never keep the test from breaking again, any
time a new table is added which contains rows when the test fixtures
start.

The only solution to making these tests runnable in their current form
is to limit the listing of tables that are searched in
archive_deleted_rows; that is, the test needs to inject a fixture into
it.  The most straightforward way to achieve this would look like this:

 @require_admin_context
-def archive_deleted_rows(context, max_rows=None):
+def archive_deleted_rows(context, max_rows=None, _limit_tablenames_fixture=None):
     """Move up to max_rows rows from production tables to the corresponding
     shadow tables.
 
@@ -5870,6 +5870,9 @@ def archive_deleted_rows(context, max_rows=None):
         if hasattr(model_class, "__tablename__"):
             tablenames.append(model_class.__tablename__)
     rows_archived = 0
+    if _limit_tablenames_fixture:
+        tablenames = set(tablenames).intersection(_limit_tablenames_fixture)
+
     for tablename in tablenames:
         rows_archived += archive_deleted_rows_for_table(context, tablename,
                                          max_rows=max_rows - rows_archived)

** Affects: nova
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Compute (nova).
https://bugs.launchpad.net/bugs/1431571

Title:
  ArchiveTestCase erroneously assumes the tables that are populated

Status in OpenStack Compute (Nova):
  New

Bug description:
  Running subsets of Nova tests or individual tests within test_db_api
  reveals a simple error in several of the tests within ArchiveTestCase.

  A test such as test_archive_deleted_rows_2_tables attempts the
  following:

  1. places six rows into instance_id_mappings
  2. places six rows into instances
  3. runs the archive_deleted_rows_ routine with a max of 7 rows to archive
  4. runs a SELECT of instances and instance_id_mappings, and confirms that only 5 remain.

  Running this test directly with PYTHONHASHSEED=random will very easily
  encounter failures such as:

  Traceback (most recent call last):
    File "/Users/classic/dev/redhat/openstack/nova/nova/tests/unit/db/test_db_api.py", line 7869, in test_archive_deleted_rows_2_tables
      self.assertEqual(len(iim_rows) + len(i_rows), 5)
    File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 350, in assertEqual
      self.assertThat(observed, matcher, message)
    File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 435, in assertThat
      raise mismatch_error
  testtools.matchers._impl.MismatchError: 8 != 5

  
  or 

  Traceback (most recent call last):
    File "/Users/classic/dev/redhat/openstack/nova/nova/tests/unit/db/test_db_api.py", line 7872, in test_archive_deleted_rows_2_tables
      self.assertEqual(len(iim_rows) + len(i_rows), 5)
    File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 350, in assertEqual
      self.assertThat(observed, matcher, message)
    File "/Users/classic/dev/redhat/openstack/nova/.tox/py27/lib/python2.7/site-packages/testtools/testcase.py", line 435, in assertThat
      raise mismatch_error
  testtools.matchers._impl.MismatchError: 10 != 5

  
  The reason is that the archive_deleted_rows() routine looks for rows in *all* tables, in *non-deterministic order*, e.g. by searching through "models.__dict__.itervalues()".   In the "8 != 5" case, there are rows present also in the instance_types table.  By PDBing into archive_deleted_rows during the test, we can see here:

  ARCHIVED 4 ROWS FROM TABLE instances
  ARCHIVED 3 ROWS FROM TABLE instance_types
  Traceback (most recent call last):
  ...
  testtools.matchers._impl.MismatchError: 8 != 5

  that is, the archiver locates seven rows just between instances and
  instance_types, then stops.  It never even gets to the
  instance_id_mappings table.

  The serious problem with the way this test is designed, is that if we
  were to make it ignore only certain tables, or make the ordering
  fixed, or anything else, that will never keep the test from breaking
  again, any time a new table is added which contains rows when the test
  fixtures start.

  The only solution to making these tests runnable in their current form
  is to limit the listing of tables that are searched in
  archive_deleted_rows; that is, the test needs to inject a fixture into
  it.  The most straightforward way to achieve this would look like
  this:

   @require_admin_context
  -def archive_deleted_rows(context, max_rows=None):
  +def archive_deleted_rows(context, max_rows=None, _limit_tablenames_fixture=None):
       """Move up to max_rows rows from production tables to the corresponding
       shadow tables.
   
  @@ -5870,6 +5870,9 @@ def archive_deleted_rows(context, max_rows=None):
           if hasattr(model_class, "__tablename__"):
               tablenames.append(model_class.__tablename__)
       rows_archived = 0
  +    if _limit_tablenames_fixture:
  +        tablenames = set(tablenames).intersection(_limit_tablenames_fixture)
  +
       for tablename in tablenames:
           rows_archived += archive_deleted_rows_for_table(context, tablename,
                                            max_rows=max_rows - rows_archived)

To manage notifications about this bug go to:
https://bugs.launchpad.net/nova/+bug/1431571/+subscriptions


Follow ups

References