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

Fix injection of ZopeLite test configuration into the global Zope configuration. #65

Merged
merged 1 commit into from
Jun 29, 2019

Conversation

thet
Copy link
Member

@thet thet commented Jun 29, 2019

Fixese #64

@thet thet requested review from datakurre and icemac June 29, 2019 17:37
@mister-roboto
Copy link

@thet 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!

@thet
Copy link
Member Author

thet commented Jun 29, 2019

@jenkins-plone-org please run jobs

Copy link
Member

@datakurre datakurre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@thet
Copy link
Member Author

thet commented Jun 29, 2019

@jenkins-plone-org please run jobs

@thet thet merged commit 5618f79 into master Jun 29, 2019
@thet thet deleted the thet-fix-64 branch June 29, 2019 18:22
@icemac icemac removed their request for review July 1, 2019 05:38
@pbauer
Copy link
Member

pbauer commented Jul 1, 2019

@thet Merging this broke all of our jenkins tests like this:

Running plone.app.theming.testing.Theming:Functional tests:
  Set up plone.testing.zope.Startup Traceback (most recent call last):
  File "/home/jenkins/.buildout/eggs/cp27m/zope.testrunner-5.0-py2.7.egg/zope/testrunner/runner.py", line 451, in run_layer
    setup_layer(options, layer, setup_layers)
  File "/home/jenkins/.buildout/eggs/cp27m/zope.testrunner-5.0-py2.7.egg/zope/testrunner/runner.py", line 787, in setup_layer
    setup_layer(options, base, setup_layers)
  File "/home/jenkins/.buildout/eggs/cp27m/zope.testrunner-5.0-py2.7.egg/zope/testrunner/runner.py", line 787, in setup_layer
    setup_layer(options, base, setup_layers)
  File "/home/jenkins/.buildout/eggs/cp27m/zope.testrunner-5.0-py2.7.egg/zope/testrunner/runner.py", line 787, in setup_layer
    setup_layer(options, base, setup_layers)
  File "/home/jenkins/.buildout/eggs/cp27m/zope.testrunner-5.0-py2.7.egg/zope/testrunner/runner.py", line 787, in setup_layer
    setup_layer(options, base, setup_layers)
  File "/home/jenkins/.buildout/eggs/cp27m/zope.testrunner-5.0-py2.7.egg/zope/testrunner/runner.py", line 787, in setup_layer
    setup_layer(options, base, setup_layers)
  File "/home/jenkins/.buildout/eggs/cp27m/zope.testrunner-5.0-py2.7.egg/zope/testrunner/runner.py", line 792, in setup_layer
    layer.setUp()
  File "/home/jenkins/workspace/pull-request-5.2/src/plone.testing/src/plone/testing/zope.py", line 311, in setUp
    self.setUpDatabase()
  File "/home/jenkins/workspace/pull-request-5.2/src/plone.testing/src/plone/testing/zope.py", line 469, in setUpDatabase
    raise Exception('You try to run plone.testing tests together with '
Exception: You try to run plone.testing tests together with ZopeTestCase tests. This will result in random failures. Convert the ZopeTestCase Tests or do not run them together

You can easily reproduce it locally like this: ./bin/test -s Products.DCWorkflow -s plone.namedfile -D

I don't really know what the warning is good for. We have not had random errors since we fixed all test-isolation-issues.

@datakurre
Copy link
Member

🙈

# NOTE: Keep that import here.
# Having it on module-level has the side effect of injecting the
# ZopeLite test configuration into the global Zope configuration.
from Testing.ZopeTestCase.ZopeLite import _patched as ZOPETESTCASEALERT
Copy link
Member Author

@thet thet Jul 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this and the following lines all together?
Looking at it, I can't make sense of importing something and then throwing an error if the import was successful.
@datakurre @pbauer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. It seems that running ZopeTestCase together with ours had side-effects that needed to be warned about. See 3268cda
I should hope that these issues are gone.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, that makes sense.
I remeber also, that was done at the Alpine City Sprint.

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

Successfully merging this pull request may close these issues.

4 participants