Issue 89 - Remove hard dependency on five.grok

collective / collective.polls

A content type, workflow, and portlet for conducting online polls, for anonymous and logged-in users

11 stars 12 forks source link

Issue 89 - Remove hard dependency on five.grok #108

Closed rodfersou closed 7 years ago

rodfersou commented 8 years ago

closes #89

Task still in progress

puittenbroek commented 7 years ago

@rodfersou Are you still working on this?

hvelarde commented 7 years ago

@puittenbroek yes, but we've been busy with other stuff; can you help?

puittenbroek commented 7 years ago

@hvelarde I'll see what I can do! There aren't that many grok elements

puittenbroek commented 7 years ago

Having some problems getting codeanalysis working locally. Working on it :)

puittenbroek commented 7 years ago

Travis issue seem related to failing Firefox.. :(

hvelarde commented 7 years ago

I have no idea what's going on; we'll need to debug.

hvelarde commented 7 years ago

@puittenbroek we're facing the same issue in other packages and I have various PR waiting for a fix.

hvelarde commented 7 years ago

@puittenbroek seems we still have some RF tests failing in Plone 4.3 probably because of an issue in latest release.

hvelarde commented 7 years ago

@puittenbroek I finally was able to ran the tests locally and I can confirm we have 3 tests failing.

we're currently working on a customer project and we're not going to be able to take a look on this until next week, probably; feel free to dig further if you can.

hvelarde commented 7 years ago

@rodfersou we're not going to be able to completely remove the dependency on five.grok until we remove the dependency on collective.z3cform.widgets.

puittenbroek commented 7 years ago

@hvelarde I'll see if I can get robot test running locally, usually my laptop (ubuntu) disagrees :)

hvelarde commented 7 years ago

@puittenbroek bin/buildout annotate is there to answer those questions: https://travis-ci.org/collective/collective.polls/jobs/218242509#L938

this is what I did locally:

download Firefox 45.8.0 ESR
extract it in /opt/firefox
run tests with PATH=/opt/firefox:$PATH bin/test

instead of running tests, I suggest you to try to replicate manually what the test is trying to achieve; these are the tests that are failing:

anonymous user should not be able to vote on a poll published on a private folder
poll tile fails rising an exception

you should run buildout with something like bin/buildout buildout:test-eggs=collective.cover[tests] in order to be able to tests all features.

puittenbroek commented 7 years ago

@hvelarde Yes, thanks I'll try the firefox thing later if I have time :) For now I fixed the two issues. Test are failing in 4.3 due to buildout not yielding the test part. I've had this in other projects as well, strange thing is that the 4.2 IS working properly. Do any other builds fail on this? Seem related to the buildout.plonetest changes?

hvelarde commented 7 years ago

@puittenbroek I made some refactor and I think I'm going to leave the utility with the old name as that's a breaking change and I would have to release as 2.0; the whole utility is somehow useless, IMO, but then we need to do a deeper refactor.

I working on fixing an issue in collective.cover that is breaking the tests here.

puittenbroek commented 7 years ago

Okay, yeah I found the utility name confusing. Especially in combination with the content class having the same naming and same interface name :)

hvelarde commented 7 years ago

IMO, there are many things in this package that are overcomplicated and I dislike that; you know, when people discover the hammer they want to hit nails everywhere ;-)

also, the scope of the PR is clear: get rid of Grok.