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

unittest2 #1882

Closed
jensens opened this issue Dec 22, 2016 · 26 comments

Comments

@jensens
Copy link
Member

@jensens jensens commented Dec 22, 2016

We're using unittest2 in many packages:

  1. we do not need it in packages targetting python 2.7 or 3 only.
  2. we do not declare it in several setup.py's as test dependency if Python 2.6 is target.

Action needed! Remove or Declare in:

  • archetypes.multilingual
  • five.pt
  • plone.app.caching
  • plone.app.collection
  • plone.app.content
  • plone.app.contentlisting
  • plone.app.contenttypes
  • plone.app.debugtoolbar
  • plone.app.discussion
  • plone.app.intid
  • plone.app.iterate
  • plone.app.linkintegrity
  • plone.app.multilingual
  • plone.app.openid
  • plone.app.portlets
  • plone.app.querystring
  • plone.app.redirector
  • plone.app.registry
  • plone.app.relationfield
  • plone.app.testing
  • plone.app.theming
  • plone.app.uuid
  • plone.app.vocabularies
  • plone.autoform
  • plone.openid See plone/plone.openid#8
  • plone.outputfilters
  • plone.portlet.collection
  • plone.portlet.static
  • plone.protect
  • plone.resource (declared)
  • plone.resourceeditor
  • plone.subrequest
  • plone.theme
  • plone.transformchain
  • Products.CMFPlone (removed)
@hvelarde

This comment has been minimized.

Copy link
Member

@hvelarde hvelarde commented Dec 22, 2016

do you have information on which packages are still being used in Python 2.6? are we properly using trove classifiers?

@jensens

This comment has been minimized.

Copy link
Member Author

@jensens jensens commented Dec 22, 2016

I would double check. Iirc @mauritsvanrees did a great job of adding classifiers, I am just not sure if it's been complete.

So I would check the sources.cfg of buildout.coredev 4.3 branch as well.

@mauritsvanrees

This comment has been minimized.

Copy link
Member

@mauritsvanrees mauritsvanrees commented Dec 23, 2016

I have added the trove classifiers a while ago. It could be that some have gotten out of sync. Best place to check is indeed coredev 4.3 sources.cfg.

@mauritsvanrees

This comment has been minimized.

Copy link
Member

@mauritsvanrees mauritsvanrees commented Dec 23, 2016

For reference, these are the extra functions in unittest2 and in the unittest of Python 2.7 that are not in Python 2.6 (ignoring some underscore methods):

addCleanup
addTypeEqualityFunc
assertDictContainsSubset
assertDictEqual
assertGreater
assertGreaterEqual
assertIn
assertIs
assertIsInstance
assertIsNone
assertIsNot
assertIsNotNone
assertItemsEqual
assertLess
assertLessEqual
assertListEqual
assertMultiLineEqual
assertNotIn
assertNotIsInstance
assertNotRegexpMatches
assertRaisesRegexp
assertRegexpMatches
assertSequenceEqual
assertSetEqual
assertTupleEqual
doCleanups
longMessage
maxDiff
setUpClass
skipTest
tearDownClass

So if a package is not using any of those, we can remove the unittest2 dependency.
And if the master branch of a package is not used on Plone 4.3, we can remove the dependency as well.

@tomgross

This comment has been minimized.

Copy link
Member

@tomgross tomgross commented Dec 23, 2016

Some packages include the dependency only for Python 2.6 (like diazo -> https://github.com/plone/diazo/blob/master/setup.py). I guess this is a good aproach for shared packages Plone 4.3 and 5.0

@gforcada

This comment has been minimized.

Copy link
Contributor

@gforcada gforcada commented Dec 26, 2016

@tomgross that's quite handy indeed, I would rather use this approach on all other packages as well, so we are sure that we are not pulling unittest2 from 2.7 onwards.

Still low hanging fruit, perfect for newcomers or when you have some hours to kill :-)

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Apr 28, 2017

Can I do this? @gforcada @jensens

@jensens

This comment has been minimized.

Copy link
Member Author

@jensens jensens commented Apr 28, 2017

@kakshay21 sure, this would be great! Do you need any directions?

@gforcada

This comment has been minimized.

Copy link
Contributor

@gforcada gforcada commented Apr 28, 2017

@kakshay21 yes please! Let us know if you have problems, either here or on community.plone.org. Go go go!

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Apr 29, 2017

Can i go little slowly on this one??
Actually, I'm in the middle of my semester end exams. But I really want to do this.
@jensens @gforcada

@gforcada

This comment has been minimized.

Copy link
Contributor

@gforcada gforcada commented Apr 29, 2017

@kakshay21 you can go as fast or as slow as you wish, one pull request at a time is the best thing to do 👍

kakshay21 added a commit to plone/archetypes.multilingual that referenced this issue Apr 30, 2017
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Apr 30, 2017
Branch: refs/heads/master
Date: 2017-04-30T15:26:09+05:30
Author: KUMAR AKSHAY (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/archetypes.multilingual@e41a419

removed unittest2 dependency

see
plone/Products.CMFPlone#1882

Files changed:
M archetypes/multilingual/tests/test_monkey.py
Repository: archetypes.multilingual
Branch: refs/heads/master
Date: 2017-04-30T15:28:37+05:30
Author: KUMAR AKSHAY (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/archetypes.multilingual@3805cdd

Update CHANGES.rst

Files changed:
M CHANGES.rst
Repository: archetypes.multilingual
Branch: refs/heads/master
Date: 2017-04-30T14:56:30+02:00
Author: Gil Forcada Codinachs (gforcada) <gil.gnome@gmail.com>
Commit: plone/archetypes.multilingual@dbb1eb9

Merge pull request #22 from plone/kakshay21-unittest

kakshay21 unittest

Files changed:
M CHANGES.rst
M archetypes/multilingual/tests/test_monkey.py
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Apr 30, 2017
Branch: refs/heads/master
Date: 2017-04-30T15:26:09+05:30
Author: KUMAR AKSHAY (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/archetypes.multilingual@e41a419

removed unittest2 dependency

see
plone/Products.CMFPlone#1882

Files changed:
M archetypes/multilingual/tests/test_monkey.py
Repository: archetypes.multilingual
Branch: refs/heads/master
Date: 2017-04-30T15:28:37+05:30
Author: KUMAR AKSHAY (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/archetypes.multilingual@3805cdd

Update CHANGES.rst

Files changed:
M CHANGES.rst
Repository: archetypes.multilingual
Branch: refs/heads/master
Date: 2017-04-30T14:56:30+02:00
Author: Gil Forcada Codinachs (gforcada) <gil.gnome@gmail.com>
Commit: plone/archetypes.multilingual@dbb1eb9

Merge pull request #22 from plone/kakshay21-unittest

kakshay21 unittest

Files changed:
M CHANGES.rst
M archetypes/multilingual/tests/test_monkey.py
kakshay21 added a commit to plone/plone.app.collection that referenced this issue Apr 30, 2017
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Apr 30, 2017
Branch: refs/heads/master
Date: 2017-04-30T18:35:16+05:30
Author: KUMAR AKSHAY (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/plone.app.collection@df2bf10

removed unittest2 dependency

plone/Products.CMFPlone#1882

Files changed:
M plone/app/collection/tests/test_collection.py
Repository: plone.app.collection
Branch: refs/heads/master
Date: 2017-04-30T18:36:52+05:30
Author: KUMAR AKSHAY (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/plone.app.collection@883f8f8

update CHANGES.rst

Files changed:
M CHANGES.rst
Repository: plone.app.collection
Branch: refs/heads/master
Date: 2017-04-30T18:37:59+05:30
Author: KUMAR AKSHAY (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/plone.app.collection@343bb53

update CHANGES.rst

Files changed:
M CHANGES.rst
Repository: plone.app.collection
Branch: refs/heads/master
Date: 2017-04-30T18:43:24+05:30
Author: kakshay21 (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/plone.app.collection@30b1065

Merge branch 'master' into kakshay21-unittest2
removing conflict

Files changed:
M CHANGES.rst
M setup.py
Repository: plone.app.collection
Branch: refs/heads/master
Date: 2017-04-30T18:44:38+05:30
Author: kakshay21 (kakshay21) <k.akshay9721@gmail.com>
Commit: plone/plone.app.collection@5675934

Merge branch 'kakshay21-unittest2'
remvoing conflict

Files changed:
M CHANGES.rst
M plone/app/collection/tests/test_collection.py
kakshay21 added a commit to plone/plone.app.collection that referenced this issue Apr 30, 2017
gforcada added a commit to plone/plone.app.collection that referenced this issue Apr 30, 2017
gforcada added a commit to plone/plone.app.collection that referenced this issue Apr 30, 2017
gforcada added a commit to plone/plone.app.collection that referenced this issue Apr 30, 2017
@jensens

This comment has been minimized.

Copy link
Member Author

@jensens jensens commented Jun 1, 2017

So many already are done! A big thanks @kakshay21

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Jun 1, 2017

Appreciate your help @jensens @gforcada

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Jun 9, 2017

I'm not able to find any repository with name "plone.portlet" neither mr.developer
mr.developer: The package 'plone.portlet' from auto-checkout has no source information.
@jensens @gforcada

@mauritsvanrees

This comment has been minimized.

Copy link
Member

@mauritsvanrees mauritsvanrees commented Jun 9, 2017

We do have plone.portlets, but that does not use unittest2 at all.
Ah, I see that plone.portlet.collection and plone.portlet.static use unittest2. I have updated the original list with them.

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Jun 10, 2017

thanks! @mauritsvanrees

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Jun 11, 2017

is there something wrong with naming plone.theme.tests ?
I'm not able to find this repo

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Jun 11, 2017

Seems like Plone is free from unittest2 now!

@mauritsvanrees

This comment has been minimized.

Copy link
Member

@mauritsvanrees mauritsvanrees commented Jun 13, 2017

plone.theme.tests would be the plone.theme package. I indeed see unittest2 in there. (I updated the main list).

@jensens

This comment has been minimized.

Copy link
Member Author

@jensens jensens commented Jun 13, 2017

I think, yeah! Now it is unittest2 free!

Last task: remove traces (also its dependencies) of unittest2 from buildout.coredev *.cfg files!

@mauritsvanrees

This comment has been minimized.

Copy link
Member

@mauritsvanrees mauritsvanrees commented Jun 14, 2017

Great work!
I see plone.formwidget.contenttree and plone.formwidget.datetime that still require unittest2 unconditionally. Well, they are not in the sources.cfg of Plone 5.1, so that may be fine.

Apart from those, when I grep through the 5.1 code, I still see real use of unittest2 in:

  • plone.app.contentlisting
  • plone.app.debugtoolbar
  • plone.openid
  • plone.resource

Of these, plone.app.debugtoolbar was missing from the original list. The other three are in there and have a checkbox claiming they have been fixed. I have updated the list to reflect the current situation.

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Jun 14, 2017

unittest2 is only mentioned in this file.
https://github.com/plone/buildout.coredev/blob/5.1/tests.cfg
there is no master branch so do i have to make changes only in 5.1 branches?
or in 5.0 too?

@jensens

This comment has been minimized.

Copy link
Member Author

@jensens jensens commented Jun 15, 2017

only in 5.1
at https://github.com/plone/buildout.coredev/blob/5.1/tests.cfg#L200-L213 there are also versions comments giving hints which packages need to get removed from version pins as well.

@kakshay21

This comment has been minimized.

Copy link
Member

@kakshay21 kakshay21 commented Mar 1, 2018

@jensens linecache2, traceback2 Do we need to remove these as well?
@mauritsvanrees I don't see any usage of unittest2 in

  • plone.app.contentlisting

  • plone.openid

  • plone.resource
    Are these removed? or I'm doing some mistake?

@gforcada

This comment has been minimized.

Copy link
Contributor

@gforcada gforcada commented Mar 1, 2018

@kakshay21 on plone.openid there is still an import for it, but as it is no longer part of plone core (not used in 5.1 nor in 5.2) it can be safely ignored. On the others you are right, there were some pull requests a few weeks/months ago that cleaned them up.

@jensens

This comment has been minimized.

Copy link
Member Author

@jensens jensens commented Mar 1, 2018

Great work! Thanks our get-rid-of-unittest2-hero @kakshay21 and all others who helped to make it happen!

@jensens jensens closed this Mar 1, 2018
@tomgross tomgross moved this from In Progress to Done in Clean-up Tasks Mar 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
6 participants
You can’t perform that action at this time.