sqlalchemy / mako

Mako Templates for Python
https://www.makotemplates.org
MIT License
353 stars 60 forks source link

Restore unit tests in classes starting with Test #365

Closed gagern closed 2 years ago

gagern commented 2 years ago

https://github.com/sqlalchemy/mako/commit/7e52b60b7dac75a3c7177e69244123c0dad9e9d9 changed tests from unittest.TestCase to pytest collection. But since then only classes ending in the word Test were considered to be tests; classes starting in Test were not. This disabled some existing tests, and while renaming these would be a viable way to restore coverage, extending the list of test class names avoids accidentally missing similar classes in the future.

The loop test classes make use of the setUp method, so in their current form they need to inherit from unittest again. Changing to pytest fixtures would be a possible future modification.

This commit adds 33 tests that had been missing before:

sqla-tester commented 2 years ago

New Gerrit review created for change 6b5ff13dd5346f3472ac93f3af0a9da172ae1c5e: https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/4043

zzzeek commented 2 years ago

it looks like 33 additional tests are running now, does that match your observations?

if you want to take a look at https://jenkins.sqlalchemy.org/job/mako_gerrit/pyv=py311/109/testReport/ that's a run, fortunately the skipped tests seem to be passing.

gagern commented 2 years ago

it looks like 33 additional tests are running now, does that match your observations?

Yes, 33 additional tests is in line with what I found locally and indicated in both the commit description and the pull request description.

zzzeek commented 2 years ago

it looks like 33 additional tests are running now, does that match your observations?

Yes, 33 additional tests is in line with what I found locally and indicated in both the commit description and the pull request description.

oh, sorry, my eyes are old and miss things frequently

sqla-tester commented 2 years ago

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/4043 has been merged. Congratulations! :)

zzzeek commented 2 years ago

thanks for this contribution!