search

plone / plone.app.testing

Testing tools for Plone-the-application
http://pypi.python.org/pypi/plone.app.testing
2 stars 7 forks source link

[fix] Preserve ZCA site for transaction hooks that depend on it #63

Open rpatterson opened 4 years ago

rpatterson commented 4 years ago

The zope.component site should not be cleared before committing transacions in case any transaction hooks depend on it.

In the test in which I ran into this issue, the transaction.commit() failed because the indexing queue was processing a plone.app.blocks.indexing.LayoutSearchableText indexer at commit-time that was trying to lookup a plone.app.blocks.layoutbehavior.ILayoutAware adapter. Since that adapter depends on behavior-derived interfaces, plone.dexterity.schema looks up a plone.dexterity.interfaces.IDexterityFTI utility for the portal_type which fails if the site isn't set which in turn yields no behavior interfaces which in turn makes the behavior-based adapter lookup fail.

In my case, this wasn't even the symptom observed. Because the commit failed silently, the changes from applying the GS profile weren't persisted including, in my case, a custom theme browser layer registration which in turn meant a custom browser view lookup failed. IOW, this silent commit failure can lead to any number of very surprising, very wrong, and hard to diagnose behaviors.

I'm not sure what changed or when, but this test worked under Python 2.7 and Plone 5.1 and I'm fairly confident that nothing changed in the testing code on "my" side that caused this and that the root change in behavior is somehow on the Plone side of things.

#

mister-roboto commented 4 years ago

@rpatterson thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

rpatterson commented 4 years ago

@jenkins-plone-org please run jobs

rpatterson commented 4 years ago

I can't understand how the CI failures are related to my changes and I don't have the time to dig deeper. Hopefully this can get merged but if not at least it's here for others having the same problem to reference.

tisto commented 4 years ago

@rpatterson Jenkins aborted the jobs after three hours. Our Jenkins instance is super slow right now, so this has nothing to do with your PR as far as I can see. If you have time, try to re-run the Jenkins job when there is low traffic on Jenkins. Sorry for the inconvenience!

rpatterson commented 4 years ago

@jenkins-plone-org please run jobs

ale-rt commented 4 years ago

@jenkins-plone-org please run jobs

jensens commented 2 years ago

Is this still a thing?