Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test-isolation-problem in test_migration_extendedtypes_vocabulary_result #251

Closed
pbauer opened this issue Aug 11, 2015 · 6 comments
Closed

Comments

@pbauer
Copy link
Sponsor Member

pbauer commented Aug 11, 2015

For fun and profit I added plone.app.linkintegrity to the "normal" test-group (plone.app.testing) in plone/buildout.coredev@940252e

The plip-build uncovered a test-isolation issue (see traceback below) originating in plone.app.contenttypes.tests.test_migration.MigrateFromATContentTypesTest.test_migration_extendedtypes_vocabulary_result. The test test_migration_extendedtypes_vocabulary_result looks pretty scary (but proves that a real thing works).

When running the tests of plone.app.linkintegrity that use the same layer (PLONE_APP_CONTENTTYPES_MIGRATION_FIXTURE) I get:

Error in test test_files_with_spaces_removal (plone.app.linkintegrity.tests.test_functional.FunctionalReferenceATTestCase)
Traceback (most recent call last):
  File "/Users/philip/workspace/buildout.python/parts/opt/lib/python2.7/unittest/case.py", line 331, in run
    testMethod()
  File "/Users/philip/workspace/coredev/src/plone.app.linkintegrity/plone/app/linkintegrity/tests/test_functional.py", line 314, in test_files_with_spaces_removal
    transaction.commit()
  File "/Users/philip/.cache/buildout/eggs/transaction-1.1.1-py2.7.egg/transaction/_manager.py", line 89, in commit
    return self.get().commit()
  File "/Users/philip/.cache/buildout/eggs/transaction-1.1.1-py2.7.egg/transaction/_transaction.py", line 329, in commit
    self._commitResources()
  File "/Users/philip/.cache/buildout/eggs/transaction-1.1.1-py2.7.egg/transaction/_transaction.py", line 443, in _commitResources
    rm.commit(self)
  File "/Users/philip/.cache/buildout/eggs/ZODB3-3.10.5-py2.7-macosx-10.4-x86_64.egg/ZODB/Connection.py", line 567, in commit
    self._commit(transaction)
  File "/Users/philip/.cache/buildout/eggs/ZODB3-3.10.5-py2.7-macosx-10.4-x86_64.egg/ZODB/Connection.py", line 623, in _commit
    self._store_objects(ObjectWriter(obj), transaction)
  File "/Users/philip/.cache/buildout/eggs/ZODB3-3.10.5-py2.7-macosx-10.4-x86_64.egg/ZODB/Connection.py", line 658, in _store_objects
    p = writer.serialize(obj)  # This calls __getstate__ of obj
  File "/Users/philip/.cache/buildout/eggs/ZODB3-3.10.5-py2.7-macosx-10.4-x86_64.egg/ZODB/serialize.py", line 422, in serialize
    return self._dump(meta, obj.__getstate__())
  File "/Users/philip/.cache/buildout/eggs/ZODB3-3.10.5-py2.7-macosx-10.4-x86_64.egg/ZODB/serialize.py", line 431, in _dump
    self._p.dump(state)
PicklingError: Can't pickle <InterfaceClass plone.app.contenttypes.tests.test_migration.IDummy>: attribute lookup plone.app.contenttypes.tests.test_migration.IDummy failed

Should we:
a) skip the test (the working code is very unlikely to change)
b) fix the test (how?)
c) add your favorite option here

@pbauer pbauer changed the title Testisolation-Problem in test_migration_extendedtypes_vocabulary_result Test-isolation-problem in test_migration_extendedtypes_vocabulary_result Aug 11, 2015
@pbauer
Copy link
Sponsor Member Author

pbauer commented Aug 11, 2015

I skipped the test for now and Oh what joy! No more test-isolation-problems when p.a.linkintegrity is part of the p.a.testing group \o/ (http://jenkins.plone.org/view/PLIPs/job/plip-698-linkintegrity/30/)
@maethu you wrote that test back in 2013. Maybe you have a idea how to fix if despite skipping it (which is a viable option IMHO).

@tisto
Copy link
Sponsor Member

tisto commented Aug 12, 2015

@pbauer This test isolation issue is most likely not caused by that single test but it is rather a sign that the test fixture or that package itself is causing trouble. Therefore skipping the test is not the best solution, since we just choose to ignore that problem until the next person runs into it. That person then needs to discover the problems that are hidden because people skipped or removed tests before. There is no easy way to fix test isolation problems and skipping tests does not solve any test isolation issue, it just makes things worse.

What I usually do to try those kind of things is I try to find two packages that have an isolation issue and run them together with:

bin/test -s plone.app.contenttypes -s plone.app.linkintegrity

When I can't see an obvious problem with the fixtures (like using functional instead of integration fixture, etc.), I start to play around with the test code to see if I can find the cause for the problem.

@maethu
Copy link

maethu commented Aug 12, 2015

@pbauer @tisto
The test I wrote registers a new adapter, so yes there is an isolation problem, since the component registry is not isolated.

I assume unregistering the adapter in the test would be a good solution, something like...

provideAdapter(DummySchemaExtender, name=u"dummy.extender")

# Clear cache
if CACHE_ENABLED:
       delattr(self.request, CACHE_KEY)

try:
        self.assertIn('dummy', doc.Schema()._names)

        vocabulary = factory(self.portal)

        self.assertEqual(1, len(vocabulary), 'Expect one entry')

        self.assertEqual("Document (1) - extended fields: 'dummy'",

finally:
       sm = getGlobalSiteManager()
       sm.unregisterAdapter(...)

Basically all tests after this one have a component registry with a dummy.extender adapter. So unregistering it would be a good practice 😄

From time to time, we (at 4teamwok) also stumbled over isolation problems regarding the component registry. Our solution was (still is) using a ComponentRegistryIsolationLayer -> https://github.com/4teamwork/ftw.testing/blob/master/ftw/testing/layer.py#L70

@pbauer
Copy link
Sponsor Member Author

pbauer commented Aug 12, 2015

@tisto exactly what I did. The traceback happens when running
./bin/test -s plone.app.contenttypes -s plone.app.linkintegrity -t test_migration_extendedtypes_vocabulary_result -t test_files_with_spaces_removal (a different order has no effect). When I move IDummy outside the test-class MigrateFromATContentTypesTest I don't get the issue anymore. Is that an acceptable solution? I guess not.

@pbauer
Copy link
Sponsor Member Author

pbauer commented Aug 12, 2015

@maethu Thanks, a pull-request would be most welcome!

@pbauer
Copy link
Sponsor Member Author

pbauer commented Aug 24, 2015

@maethu ping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants