Skip to content

Restore unit tests in classes starting with Test#365

Closed
gagern wants to merge 1 commit intosqlalchemy:mainfrom
gagern:test_loop
Closed

Restore unit tests in classes starting with Test#365
gagern wants to merge 1 commit intosqlalchemy:mainfrom
gagern:test_loop

Conversation

@gagern
Copy link
Copy Markdown
Contributor

@gagern gagern commented Aug 19, 2022

7e52b60 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:

  • 25 methods in 4 classes from test_loop.py
  • 2 methods in TestTemplateAPI from test_template.py
  • 6 methods in TestTGPlugin from test_tgplugin.py

sqlalchemy@7e52b60
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:
* 25 methods in 4 classes from test_loop.py
* 2 methods in TestTemplateAPI from test_template.py
* 6 methods in TestTGPlugin from test_tgplugin.py
Copy link
Copy Markdown
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 6b5ff13 of this pull request into gerrit so we can run tests and reviews and stuff

Copy link
Copy Markdown
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 6b5ff13 of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Copy Markdown
Collaborator

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

@zzzeek
Copy link
Copy Markdown
Member

zzzeek commented Aug 20, 2022

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
Copy link
Copy Markdown
Contributor Author

gagern commented Aug 20, 2022

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
Copy link
Copy Markdown
Member

zzzeek commented Aug 20, 2022

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
Copy link
Copy Markdown
Collaborator

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

@zzzeek
Copy link
Copy Markdown
Member

zzzeek commented Aug 20, 2022

thanks for this contribution!

artdogma added a commit to artdogma/pymako that referenced this pull request Aug 27, 2025
sqlalchemy/mako@7e52b60 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:
* 25 methods in 4 classes from `test_loop.py`
* 2 methods in `TestTemplateAPI` from `test_template.py`
* 6 methods in `TestTGPlugin` from `test_tgplugin.py`

Closes: #365
Pull-request: sqlalchemy/mako#365
Pull-request-sha: 6b5ff13dd5346f3472ac93f3af0a9da172ae1c5e

Change-Id: I744b8c6f28cf485c07cd30a11c328ea7391c7d3b
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.

3 participants