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.

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

Templates : Correction de variables manquantes #3257

Merged
merged 2 commits into from
Oct 25, 2023
Merged

Templates : Correction de variables manquantes #3257

merged 2 commits into from
Oct 25, 2023

Conversation

rsebille
Copy link
Contributor

Pourquoi ?

Car personne ne semble vouloir les corriger.
Lancer pytest -v tests/www/approvals_views/test_detail.py résulte en erreur car des variables ne sont pas définies, je ne sais pas pourquoi elle ne sont pas sur la CI mais c'est peut-être dû au fait que ça soit des TestCase 🤷.

@rsebille rsebille added the no-changelog Ne doit pas figurer dans le journal des changements. label Oct 24, 2023
@rsebille rsebille requested a review from xavfernandez October 24, 2023 11:02
@rsebille rsebille self-assigned this Oct 24, 2023
@xavfernandez
Copy link
Contributor

Cela m'embête qu'on ne comprenne pas pourquoi cela passe en CI et pas en local...

@hellodeloo
Copy link
Contributor

hellodeloo commented Oct 24, 2023 •
edited
Loading

@rsebille je vois qu'il y a encore des {{ base_url }} présent dans /itou/templates/apply/email/new_for_job_seeker_body.txt. J'imagine que ce n'est pas génant car sur un fichier de template d'email donc surement pas testé mais bon, je le signale quand même ;)

@rsebille
Copy link
Contributor Author

@xavfernandez Je n'ai rien trouvé qui explique ce comportement, faudrait tester sur la CI vu que ça semble venir de là bas mais j'ai pas trop le temps pour le moment 😩.

@hellodeloo Eux ne vont pas poser problème car la variable est définis dans le contexte, et ici elle est nécessaire : https://github.com/betagouv/itou/blob/eb7ca24bd99252c957bcab756b03fd8f7866fe8c/itou/job_applications/models.py#L1031-L1036
Et le mail est testé ;) https://github.com/betagouv/itou/blob/eb7ca24bd99252c957bcab756b03fd8f7866fe8c/tests/job_applications/tests.py#L716-L747

@xavfernandez
Copy link
Contributor

Je creuse

@xavfernandez
Copy link
Contributor

xavfernandez commented Oct 24, 2023 •
edited
Loading

Je pense que cela vient de https://github.com/pytest-dev/pytest-django/blob/master/pytest_django/plugin.py#L650 qui ne remet jamais fail à True:

  • pytest tests/www/apply/test_geiq.py tests/www/approvals_views/test_detail.py tests/www/approvals_views/test_display.py ne plante pas car test_geiq.py a passé fail à False
  • pytest tests/www/approvals_views/test_detail.py tests/www/approvals_views/test_display.py tests/www/apply/test_geiq.py plante bien

Je fais une PR dans ce sens.

@rsebille
Copy link
Contributor Author

Oh god 🤦.
En plus je me suis fait la réflexion "C'est comme si l'option n'était pas prise en compte", mais j'ai cherché au niveau du pytest.ini, ci.yml, de l'env, etc. Pas dans la mark...

@xavfernandez
Copy link
Contributor

xavfernandez commented Oct 24, 2023 •
edited
Loading

https://github.com/pytest-dev/pytest-django/pull/1076/files

Je vais faire une PR corrigeant le soucis chez nous et tu pourras rebaser cette PR dessus je pense ?

Edited: cf #3262

@rsebille
templates: Fix a missing, and unnecessary, base_url variable
3f254bc
@rsebille
tests.www.approvals: Tighten pytest.mark.ignore_template_errors
49c2d24 
We could circumvent the error but the tests are kind of designed to not
have a job application so it's maybe not a terrible idea to ignore the
error for those specific test cases to not add some complexity elsewhere.
@rsebille rsebille force-pushed the rsebille/undef_tmpl_var branch from 3361996 to 49c2d24 Compare October 25, 2023 09:15
@rsebille rsebille added this pull request to the merge queue Oct 25, 2023
Merged via the queue into master with commit 6fc624c Oct 25, 2023
5 checks passed
@rsebille rsebille deleted the rsebille/undef_tmpl_var branch October 25, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xavfernandez xavfernandez xavfernandez approved these changes

Assignees

@rsebille rsebille

Labels
no-changelog Ne doit pas figurer dans le journal des changements.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rsebille @xavfernandez @hellodeloo