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

Python 3.6 new #50

Merged
merged 57 commits into from
Oct 17, 2018
Merged

Python 3.6 new #50

merged 57 commits into from
Oct 17, 2018

Conversation

jensens
Copy link
Member

@jensens jensens commented Jul 3, 2018

So, this is all from #48, plus rebase with master, plus additional changes from last sprints. sigh

It is what is used in Plone 5.2 coredev buildout right now.

In order to merge at least plone.app.testing needs also to be ready.

This really should be addressed and get merged, any input on open todos welcome!
cc pbauer @icemac @loechel

Closes #48

Michael Howitz and others added 30 commits July 3, 2018 14:30
This implies testing them separately.

TODO:
- remove duplicate code
- bring back z2 for BBB
Reusing the code from wsgi.py in zserver.py, so it can be easily deleted when zserver is no longer supported.
But we use the same names used in zserver.py.
The `WSGIServer` fixture is still to be developed.
[skip ci]
(Some plone tests do this.)
[skip ci]
This shows more clearly the purpose of the module.
Michael Howitz and others added 3 commits July 3, 2018 14:30
Without these changes the ZServer layer implicitly uses the Zope app in some
places. This breaks tests later on which run on the Zope layer with the
following traceback:

    Traceback (most recent call last):
      ...
      File ".../plone/testing/zope.py", line 244, in zopeApp
        app = addRequestContainer(Zope2.app(connection), environ=environ)
      File ".../Zope2/__init__.py", line 55, in app
        return bobo_application(*args, **kw)
    TypeError: 'NoneType' object is not callable
@jensens
Copy link
Member Author

jensens commented Jul 3, 2018

Lets see what happens with a PR job on 5.2 with

https://github.com/plone/plone.testing/pull/50
https://github.com/plone/plone.app.testing/pull/49

@icemac
Copy link

icemac commented Jul 4, 2018

@jensens wrote:

Lets see what happens with a PR job on 5.2 with

https://github.com/plone/plone.testing/pull/50
https://github.com/plone/plone.app.testing/pull/49

The result is a bit disappointing: 1185 broken tests. This looks like a problem on a deeper level. I currently do not have the time and energy to look into it.

@jensens
Copy link
Member Author

jensens commented Jul 4, 2018

@icemac indeed, no problem, I will take a look soon.

This was referenced Sep 27, 2018
davisagli and others added 5 commits October 2, 2018 22:09
This was a fun one.
The zope.security.checker module has a module-global _checkers dict.
This layer tries to manage it as a stack, where pushing to the stack
copies the current dict and popping from the stack sets
zope.security.checker._checkers back to the copy that was stored.

But, it turns out that _checkers is imported from a C extension module,
and that module keeps a pointer to the original dict that is not
affected by the plone.testing stack pop. So subsequent lookups continue
to use the old dict.

To fix this I changed the stack pop operation to clear and update the
dict instead of replacing it, so that the pointer in the C module
is still valid.

There was a similar but different problem with the pure-Python implementation:
it sets _getChecker = _checkers.get initially and _getChecker was not updated
by the stack pop. My workaround should also deal with this case.
@davisagli davisagli force-pushed the py3_disallow_commits branch from 7a5d836 to c7c080b Compare October 6, 2018 18:31
@jensens jensens merged commit c7de924 into master Oct 17, 2018
@jensens jensens deleted the py3_disallow_commits branch October 17, 2018 09:04
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.

7 participants