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

Make sure InvalidVarException.fail is reset #1076

Merged

Conversation

xavfernandez
Copy link
Contributor

after a test using ignore_template_errors marker.

Otherwise, the first test set fail to False, never to be changed again.

Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Good catch @xavfernandez, I'm surprised no one had noticed this bug before.

I left a suggestion for simplifying the test.

Also, scrolling up a bit I see that _fail_for_invalid_template_variable has a similar issue, although it's less consequential since it's a session fixture. But it would be good to fix it as well.

def test_invalid_template_variable_marker_cleanup(django_testdir) -> None:
django_testdir.create_app_file(
"""
from django.urls import path
Copy link
Member

Choose a reason for hiding this comment

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

I think it will be easier to have the tests render the template directly using e.g. render_to_string instead of setting up views and requests and everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test updated

after a test using ignore_template_errors marker
@xavfernandez xavfernandez force-pushed the fix_template_string_if_invalid_marker branch from bed7b43 to 787ceef Compare October 26, 2023 07:49
@xavfernandez
Copy link
Contributor Author

I also added cleanup to _fail_for_invalid_template_variable as you suggested

@xavfernandez xavfernandez force-pushed the fix_template_string_if_invalid_marker branch from c9c29aa to e71523a Compare October 26, 2023 08:06
@xavfernandez xavfernandez force-pushed the fix_template_string_if_invalid_marker branch from e71523a to 86a93e7 Compare October 26, 2023 08:18
@xavfernandez xavfernandez force-pushed the fix_template_string_if_invalid_marker branch from 714dd8f to e9db1b7 Compare October 26, 2023 08:43
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks!

@bluetech bluetech merged commit 4e1906a into pytest-dev:master Oct 26, 2023
14 of 15 checks passed
@xavfernandez xavfernandez deleted the fix_template_string_if_invalid_marker branch October 26, 2023 19:24
xavfernandez added a commit to gip-inclusion/les-emplois that referenced this pull request Nov 6, 2023
pytest-dev/pytest-django#1076 has been merged &
released (in 4.6.0).
github-merge-queue bot pushed a commit to gip-inclusion/les-emplois that referenced this pull request Nov 6, 2023
pytest-dev/pytest-django#1076 has been merged &
released (in 4.6.0).
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.

2 participants