Skip to content

Added tests for assertRedirects() when following a redirect. #19621

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 17, 2025
Merged

Added tests for assertRedirects() when following a redirect. #19621

merged 2 commits into from
Jul 17, 2025
+189 −149

Conversation

cliff688
Copy link
Member

@cliff688 cliff688 commented Jul 6, 2025 •
edited
Loading

Trac ticket number

Refs ticket-4476 and ticket-35728

Branch description

I could remove the assertions:

django/django/test/testcases.py

Lines 428 to 437 in b2407e4

self.assertEqual(
response.redirect_chain[0][1],
status_code,
msg_prefix
+ (
"Initial response didn't redirect as expected: Response code was "
"%d (expected %d)"
)
% (response.redirect_chain[0][1], status_code),
)
and

django/django/test/testcases.py

Lines 441 to 450 in b2407e4

self.assertEqual(
response.status_code,
target_status_code,
msg_prefix
+ (
"Response didn't redirect as expected: Final Response code was %d "
"(expected %d)"
)
% (response.status_code, target_status_code),
)
without any test failures.

Additional commit splits monolithic test_contains() test into smaller, more focused tests.

@github-actions github-actions bot added the no ticket Based on PR title, no linked Trac ticket label Jul 6, 2025
nessita
nessita reviewed Jul 16, 2025
View reviewed changes
Copy link
Contributor

@nessita nessita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @cliff688, added some suggestions! LMKWYT 🌟

@cliff688 cliff688 force-pushed the assertion-msgs-tests branch from 7adcff8 to c4a3ac6 Compare July 17, 2025 12:59
cliff688 added 2 commits July 17, 2025 16:28
@cliff688 @nessita
Split monolithic AssertContainsTests.test_contains() into focused tests.
da89bc3 
Added a local helper to test assertion messages with and without a given
msg_prefix.
@cliff688 @nessita
Refs django#4476 -- Added tests for assertRedirects() when following …
2c8e151 
…redirect chains.

Thanks Natalia Bidart for the review.
@nessita nessita force-pushed the assertion-msgs-tests branch from c4a3ac6 to 2c8e151 Compare July 17, 2025 19:33
@nessita nessita merged commit 04e813c into django:main Jul 17, 2025
31 of 32 checks passed
@cliff688 cliff688 deleted the assertion-msgs-tests branch July 18, 2025 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nessita nessita nessita approved these changes

Assignees
No one assigned
Labels
no ticket Based on PR title, no linked Trac ticket
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cliff688 @nessita