Help needed to fix hundreds of failing tests

98 views
Skip to first unread message

Peter Inglesby

unread,
Oct 10, 2020, 3:23:44 PM10/10/20
to django-d...@googlegroups.com
Hi folks,

I've authored PR 12417 (Fixed 31235 -- Updated assertQuerysetEqual to compare querysets directly) which has added a new RemovedInDjango41Warning... which causes hundreds of test failures.

Unfortunately I have no spare time at the moment to resolve these.  Would anybody on this list like to take the PR off my hands?

Thanks!

Peter.

Hasan Ramezani

unread,
Oct 10, 2020, 6:20:55 PM10/10/20
to django-d...@googlegroups.com
Hi Peter,

Thanks for your effort there. I can take it and continue working on it.

Best regards,

Hasan

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAENJrPnduNHNB9Fr7_5m40LD7%2B_P%3D8f45BuK6dx3cEROE2_jYQ%40mail.gmail.com.

Carles Pina i Estany

unread,
Oct 10, 2020, 6:59:07 PM10/10/20
to django-d...@googlegroups.com

Hi,

On Oct/10/2020, Peter Inglesby wrote:

I remember reading the original thread...

> I've authored PR 12417 <https://github.com/django/django/pull/12417> (Fixed
> 31235 <https://code.djangoproject.com/ticket/31235> -- Updated
> assertQuerysetEqual to compare querysets directly) which has added a new
> RemovedInDjango41Warning... which causes hundreds of test failures.
>
> Unfortunately I have no spare time at the moment to resolve these. Would
> anybody on this list like to take the PR off my hands?

I was now reading the updated documentation and code. Just to help
someone to fix the unit tests, is this what you thought that should be
done?

----------
diff --git a/tests/transactions/tests.py b/tests/transactions/tests.py
index 2ac2f8cc84..f265339c4f 100644
--- a/tests/transactions/tests.py
+++ b/tests/transactions/tests.py
@@ -163,12 +163,12 @@ class AtomicTests(TransactionTestCase):
def test_reuse_commit_rollback(self):
atomic = transaction.atomic()
with atomic:
- Reporter.objects.create(first_name="Tintin")
+ reporter = Reporter.objects.create(first_name="Tintin")
with self.assertRaisesMessage(Exception, "Oops"):
with atomic:
Reporter.objects.create(first_name="Haddock")
raise Exception("Oops, that's his last name")
- self.assertQuerysetEqual(Reporter.objects.all(), ['<Reporter: Tintin>'])
+ self.assertQuerysetEqual(Reporter.objects.all(), [reporter])

def test_reuse_rollback_commit(self):
atomic = transaction.atomic()
----------

(and another 257... with my DB setup)

It does fix the unit test, I don't know if this is what you intended.

Or, a smaller fix:
self.assertQuerysetEqual(Reporter.objects.all(), ['Tintin'], transform=str)

Which at least doesn't require thinking of a variable name (and it might
be easier if the objects are not created by the surrounding code, I
haven't checked if this happens often or not).

Or, did I take the wrong side of the stick and you intended the fix to be
different?

Have a good weekend,

--
Carles Pina i Estany
https://carles.pina.cat

Peter Inglesby

unread,
Oct 12, 2020, 3:35:21 PM10/12/20
to django-d...@googlegroups.com, hasa...@gmail.com
Thanks Hasan for offering to help.

Either of Carles's approaches would work -- I don't have a strong view.  Making the smaller fix (ie adding transform=str every time assertQuerysetEqual fails) would require the least thinking.  But it may miss some good opportunities to refactor or simplify the tests.
Reply all
Reply to author
Forward
0 new messages