fallback to unittest when unittest2 is not available #340

pgajdos · 2020-06-03T09:02:44Z

unittest2 is not neccessary in python3

…t neccessary in python3

mcepl · 2020-06-22T06:29:53Z

@pgajdos Well, the question remains: why is it needed for Python 2.7? (it would be nice to mention it in changelog/spec file)? Is it really needed?

pgajdos · 2020-06-25T08:11:07Z

@mcepl

[    7s] ____ SerializableTransformTest.test_encode_with_custom_repr_returns_object _____
[    7s] 
[    7s] self = <rollbar.test.test_serializable_transform.SerializableTransformTest testMethod=test_encode_with_custom_repr_returns_object>
[    7s] 
[    7s]     def test_encode_with_custom_repr_returns_object(self):
[    7s]         class CustomRepr(object):
[    7s]             def __repr__(self):
[    7s]                 return {'hi': 'there'}
[    7s]     
[    7s]         start = {'hello': 'world', 'custom': CustomRepr()}
[    7s]     
[    7s]         serializable = SerializableTransform(whitelist_types=[CustomRepr])
[    7s]         result = transforms.transform(start, serializable)
[    7s] >       self.assertRegex(result['custom'], "<class '.*CustomRepr'>")
[    7s] E       AttributeError: 'SerializableTransformTest' object has no attribute 'assertRegex'
[    7s]

I think it is lost time to deal with it more as 2.7 is eol.

mcepl · 2020-06-25T08:30:54Z

import six

six.assertRegex(self, result['custom'], "<class '.*CustomRepr'>")

I think it is lost time to deal with it more as 2.7 is eol.

Not for SLE (although python-rollbar is not in SLE).

pgajdos · 2020-06-25T12:42:13Z

@mcepl

import six

six.assertRegex(self, result['custom'], "<class '.*CustomRepr'>")

Unfortunately, that does not work for me so.

I think it is lost time to deal with it more as 2.7 is eol.

Not for SLE (although python-rollbar is not in SLE).

I do not discuss downstream matters in upstream issues. (I do not see the issue with above patch in SLE, though, perhaps bit of reasoning would help me understand the problem.)

letaniaferreira

LGTM

mrunalk · 2020-07-15T19:26:41Z

rollbar/test/__init__.py

@@ -1,9 +1,12 @@
-import unittest2
+try:
+ import unittest2 as unittest


Thanks for your change. I don't think we even need to fallback on unittest2 as Python 2+ has been EOLed. Just move ahead with replacing it to unittest.

Well, it doesn’t save me. We have rollbar in Leap 15.* so we still build python2- packages. I don’t want to knowingly break python2 compatibility if I can help it. Do you know rollbar functionality available only in Python 3.* or unittest2 respectively?

@mcepl, Thank you for your feedback. We are definitely not removing the support for python2 yet. If we can replace unittest2 with unittest without breaking existing tests and builds that will be great.

pgajdos · 2020-07-16T09:33:42Z

Done.

mrunalk · 2020-07-18T00:20:25Z

.travis.yml

@@ -2,16 +2,6 @@ sudo: false
 language: python
 matrix:
 include:
- - python: "2.7"


We are not removing support for Python2+ yet so please revert this part. May I know your motivation behind supporting unittest?

@pgajdos ???

mrunalk · 2020-07-18T01:37:26Z

rollbar/test/__init__.py

@@ -1,9 +1,12 @@
-import unittest2
+try:
+ import unittest2 as unittest


@mcepl, Thank you for your feedback. We are definitely not removing the support for python2 yet. If we can replace unittest2 with unittest without breaking existing tests and builds that will be great.

https://build.opensuse.org/request/show/821667 by user mcepl + dimstar_suse - Upstream doesn't care about unittest2 at all (gh#rollbar/pyrollbar#340), we can just ignore it. Adjust python-rollbar-no-unittest2.patch accordingly.

pgajdos · 2020-08-04T12:50:20Z

Hmm, @mrunalk and @mcepl: so should I actually revert to (original) cdfc35b?

I do not insist on making this change myself, so feel free to do the change yourself, if you wish -- I can just file an issue, if you think it would be better.

mcepl · 2020-08-04T17:34:21Z

Hmm, @mrunalk and @mcepl: so should I actually revert to (original) cdfc35b?

I do not insist on making this change myself, so feel free to do the change yourself, if you wish -- I can just file an issue, if you think it would be better.

I still haven’t got a reply to my answer what capabilities of python3/unittest2 is used in the test suite. Do we need unittest2 (with Python 2.7) at all?

pgajdos · 2020-08-05T08:41:24Z

@mcepl

See #340 (comment).

mrunalk · 2020-08-05T20:23:04Z

Hmm, @mrunalk and @mcepl: so should I actually revert to (original) cdfc35b?

I do not insist on making this change myself, so feel free to do the change yourself, if you wish -- I can just file an issue, if you think it would be better.

I am OK removing 'unittest2' but Not OK with removing support for python2 at this time. Alternatively I can merge your original patch of 'fallback to unittest when unittest2 is not available' if you can provide some explanation on what is motivating on this change that will help understand the context.

mcepl · 2020-08-06T06:49:29Z

I am OK removing 'unittest2' but Not OK with removing support for python2 at this time.

I completely agree, see the linked pull request.

https://build.opensuse.org/request/show/824631 by user mcepl + dimstar_suse - Replace self.assertRegex with six.assertRegex in python-rollbar-no-unittest2.patch to finally unrequire unittest2 (gh#rollbar/pyrollbar#340). - Update the patch according to the pending pull request gh#rollbar/pyrollbar#346.

fallback to unittest when unittest2 is not available; unittest2 is no…

cdfc35b

…t neccessary in python3

vselvarajijay requested a review from letaniaferreira July 10, 2020 18:34

letaniaferreira requested review from mrunalk and removed request for letaniaferreira July 10, 2020 18:34

vselvarajijay requested a review from letaniaferreira July 10, 2020 18:38

letaniaferreira approved these changes Jul 10, 2020

View reviewed changes

mrunalk suggested changes Jul 15, 2020

View reviewed changes

pgajdos added 2 commits July 16, 2020 11:19

remove unittest2 entirely

20041b6

remove 2.7

10ec72f

mrunalk reviewed Jul 18, 2020

View reviewed changes

mcepl mentioned this pull request Aug 5, 2020

Remove all vestiges of unittest2. #346

Closed

mcepl mentioned this pull request Feb 23, 2023

Remove unittest2 #419

Merged

brianr mentioned this pull request Feb 23, 2023

Replace unittest2 with unittest #420

Merged

12 tasks

brianr closed this in bcc024e Feb 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fallback to unittest when unittest2 is not available #340

fallback to unittest when unittest2 is not available #340

pgajdos commented Jun 3, 2020

mcepl commented Jun 22, 2020

pgajdos commented Jun 25, 2020

mcepl commented Jun 25, 2020 •

edited

Loading

pgajdos commented Jun 25, 2020

letaniaferreira left a comment

mrunalk Jul 15, 2020

mcepl Jul 16, 2020

mrunalk Jul 18, 2020

pgajdos commented Jul 16, 2020

mrunalk Jul 18, 2020 •

edited

Loading

mcepl Jul 18, 2020

mrunalk Jul 18, 2020

pgajdos commented Aug 4, 2020

mcepl commented Aug 4, 2020

pgajdos commented Aug 5, 2020

mrunalk commented Aug 5, 2020

mcepl commented Aug 6, 2020

fallback to unittest when unittest2 is not available #340

fallback to unittest when unittest2 is not available #340

Conversation

pgajdos commented Jun 3, 2020

mcepl commented Jun 22, 2020

pgajdos commented Jun 25, 2020

mcepl commented Jun 25, 2020 • edited Loading

pgajdos commented Jun 25, 2020

letaniaferreira left a comment

Choose a reason for hiding this comment

mrunalk Jul 15, 2020

Choose a reason for hiding this comment

mcepl Jul 16, 2020

Choose a reason for hiding this comment

mrunalk Jul 18, 2020

Choose a reason for hiding this comment

pgajdos commented Jul 16, 2020

mrunalk Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

mcepl Jul 18, 2020

Choose a reason for hiding this comment

mrunalk Jul 18, 2020

Choose a reason for hiding this comment

pgajdos commented Aug 4, 2020

mcepl commented Aug 4, 2020

pgajdos commented Aug 5, 2020

mrunalk commented Aug 5, 2020

mcepl commented Aug 6, 2020

mcepl commented Jun 25, 2020 •

edited

Loading

mrunalk Jul 18, 2020 •

edited

Loading