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.

Sign up for GitHub

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

Jump to bottom

Fix zope interface 5.1.x fieldset problem #253

Merged
merged 4 commits into from
Nov 11, 2020
Merged

Fix zope interface 5.1.x fieldset problem #253

merged 4 commits into from
Nov 11, 2020

Conversation

mauritsvanrees
Copy link
Member

See issue #252.
This does not fix anything, but it should show the errors on Travis.

@coveralls
Copy link

coveralls commented Nov 3, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.005%) to 85.103% when pulling 4ac8018 on maurits/zope-interface-51x-fieldset-problem into 6892b29 on master.

@mauritsvanrees mauritsvanrees changed the title WIP: show zope interface 5.1.x fieldset problem Fix zope interface 5.1.x fieldset problem Nov 4, 2020
@mauritsvanrees
Copy link
Member Author

Update: this PR now contains a fix.

Problem was that the superAdapter called providedBy(adapter).declared and expected this to be a list containing a single IValidator. This is true in Plone 5.2.2 with zope.interface 5.0.2. This is also true with zope.interface 5.1.2 for the FieldExtenderValidator. But for the GroupFieldExtenderValidator it is empty. So the superAdapter finds no validator.
It looks like providedBy(adapter).interfaces() works both times and for both zope.interface versions. Another part of the superAdapter was already using this, so it seems safe.

I have temporarily changed Travis to test on 5.2.2 and 5.2.3-pending, so we test both old and new zope.interface. Works locally. I guess it will work on 5.0 and 5.1 as well, but Travis will tell us.

@mauritsvanrees mauritsvanrees mentioned this pull request Nov 4, 2020
Closed
@mauritsvanrees
Travis: test on both 5.2.x and 5.2.3-pending.
33f694b 
On those versions, pin check-manifest to 0.44 to prevent getting version conflicts for `build`, `pep517` and `virtualenv`.

We want to test with newer and older zope.interface versions, for #252
5.2.x is currently 5.2.2, with zope.interface 5.0.2.
5.2.3-pending has zope.interface 5.1.2.

This will fail once 5.2.3-pending is out of pending, but that is just a signal that I can clean this Travis config up again.
@mauritsvanrees
Fix validators in field sets with zope.interface 5.1+.
8ab369e 
This fixes #252.
@mauritsvanrees
Fixed email tests with newer Products.MailHost.
a04596e 
They pass now on Plone 5.2.2 and 5.2.3 with Python 3.8.
@mauritsvanrees mauritsvanrees force-pushed the maurits/zope-interface-51x-fieldset-problem branch from 7e5a448 to a04596e Compare November 5, 2020 12:40
@mauritsvanrees
Copy link
Member Author

The tests on Plone 5.2.3 failed because it has a newer Products.MailHost, which means messages are bytes, not text. I updated the tests for that. No changes in real code needed.
I squashed and rebased, so this PR currently has three commits:

  • buildout and Travis
  • the real validator fixes that this PR is mainly about
  • mail fixes in tests

mauritsvanrees added a commit to plone/buildout.coredev that referenced this pull request Nov 5, 2020
@mauritsvanrees
Update pep517 to version that works with latest check-manifest.
fe1f8bd 
Updated check-manifest too.
It depends on 'build', added that too.
I ran into version conflicts with the easyform buildout, see collective/collective.easyform#253
@mauritsvanrees mauritsvanrees mentioned this pull request Nov 5, 2020
Merged
@mauritsvanrees
Copy link
Member Author

The Travis links show as 'pending', but when you follow those links, you will see the build is green.
Ready for review.

@mauritsvanrees
Copy link
Member Author

No reaction after almost a week. This package is not a reviewer-rich environment. ;-)
Build is green, I merge.

@mauritsvanrees mauritsvanrees merged commit 5585125 into master Nov 11, 2020
@mauritsvanrees mauritsvanrees deleted the maurits/zope-interface-51x-fieldset-problem branch November 11, 2020 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djay djay Awaiting requested review from djay

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@mauritsvanrees @coveralls