django icon indicating copy to clipboard operation
django copied to clipboard

Added further tests demonstrating usage of functools.partial as a view and the effects thereof

Open kezabelle opened this issue 2 years ago • 7 comments

As far as I could tell from glancing around, and a bit of pdb jiggery-pokery, these situations weren't explicitly covered. I may be wrong, and I just couldn't find them, though.

The tests pass because this is what is currently happening, but it's debatable that they should pass.

  • first commit covers using path('url/part', partial(myview, x=1)) that is, using a partially applied function without also giving the pattern a nice name. Without using update_wrapper the 'proper' (debatable!) name gets swallowed.
  • second commit is basically the same thing, but for the displaying of the view name in the technical 404.
  • third commit is the same, but for the new Raised during being shown on the technical 500 (yay!)

Between ResolverMatch.__repr__ and URLPattern.lookup_str (and the tests of those) it's clear that partials are supported, and perhaps we can improve their output, but at the very least we can document their current output.

This vaguely references tickets 33396, 22756, probably 22486 and sort of 33425.

kezabelle avatar Jan 12 '22 16:01 kezabelle

Well that's puzzling ... some of the 3.10 builds pass and others don't?! and view_tests.tests.test_debug.ExceptionReporterTests.test_reporting_frames_for_cyclic_reference failed but only for postgres,bionic-pr,python3.10 ...

Colour me confused.


Edit: Under my 3.10 install, test_partial_classbased_technical_404 and test_partial_technical_404 output functools.partial rather than partial ... but 2 of the CI runs for 3.10 completed successfully, as did all the 3.8 and 3.9 ones. I am no less confused.

kezabelle avatar Jan 12 '22 17:01 kezabelle

but 2 of the CI runs for 3.10 completed successfully

Those two 3.10 runs that were successful were for mysql_gis. They only ran the tests under gis_tests.

ngnpope avatar Jan 13 '22 10:01 ngnpope

Those two 3.10 runs that were successful were for mysql_gis. They only ran the tests under gis_tests.

Ah, of course! Thank you for that. That still leaves me scratching my head about test_reporting_frames_for_cyclic_reference but perhaps I can hope that was a transient failure; we'll find out when I try and fixup :)

kezabelle avatar Jan 13 '22 10:01 kezabelle

Aha, happily for me, d05ab13c56849ed09d02c1f188b7f47f0c090a2a landed and made the output consistently functools.partial across versions. And the unrelated failure was maybe transient or maybe leaky because of one of the other failures. The only failure now is flake8 being annoyingly whiny ;)

kezabelle avatar Jan 13 '22 13:01 kezabelle

@kezabelle Thanks :+1: Can you describe the next step? Do you want to improve tests coverage? or maybe fix a :bug: (in which case a new ticket is required)?

felixxm avatar Aug 31 '22 11:08 felixxm

I definitely need to revisit this, leave it with me? Primarily I think the test coverage was to show the "current" behaviour that was otherwise implicit in some sections (e.g. the technical debug pages) and make concrete the expectations of using partials, and perhaps from there improve/change that.

However, the PR's description alone doesn't remind me of enough of the context, so no-one else could be expected to get it. Possibly that means it deserves a ticket, or at least a rebase and re-describe of the overall purpose.

kezabelle avatar Sep 01 '22 08:09 kezabelle

Meanders of mind :brain: :wink: Take your time, I'm just cleaning up.

felixxm avatar Sep 01 '22 08:09 felixxm

@kezabelle Closing for now. Please feel-free to send a new PR, thanks!

felixxm avatar Oct 11 '22 03:10 felixxm