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

Fix test for changed error message from newer Django (djmain) #1486

Merged
merged 3 commits into from
Sep 6, 2024

Conversation

n2ygk
Copy link
Member

@n2ygk n2ygk commented Sep 5, 2024

Fixes #1482

Description of the Change

Older Django has one error message that the failure test was looking for. Newer Django (main branch) has a different error message, so check for both.

Checklist

  • PR only contains one change (considered splitting up PR)
  • unit-test added
  • documentation updated
  • CHANGELOG.md updated (only for user relevant changes)
  • author name in AUTHORS

@n2ygk n2ygk requested a review from a team September 5, 2024 18:11
@n2ygk n2ygk added this to the Future milestone Sep 5, 2024
self.assertIn("user", output_str)
self.assertIn("783", output_str)
# newer Django (>5.1) changes the error message from "does not exist" to "is not a valid choice"
self.assertTrue(
Copy link
Contributor

Choose a reason for hiding this comment

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

would something like the following be more self documenting?

import django 

if django.VERSION <= (5,1):
   self.assertIn("does not exist", output.getvalue())
else:
   self.assertIn("is not a valid choice", output.getvalue())

Copy link
Member Author

Choose a reason for hiding this comment

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

I had to make it a test for < (5,2) because (5,1) is not <= (5, 1, 1, 'final', 0) . I suppose the alternative would be to take the first 2 elements of the tuple, but I think this is readable enough....

@n2ygk n2ygk force-pushed the bug/1482/assertion branch from 8aef61f to 6bf75c0 Compare September 6, 2024 14:14
@dopry dopry merged commit f220235 into jazzband:master Sep 6, 2024
19 checks passed
@n2ygk n2ygk modified the milestones: Future, Release 3.0.1 Sep 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

py{310,311,312}-djmain tests.test_commands.CreateApplicationTest failing
2 participants