pytest doesn't run on a fresh clone. Nothing tells it which Django settings to load:
$ pytest
ImproperlyConfigured: Requested setting ..., but settings are not configured
no tests collected, 7 errors
This commits the small info needed to pyproject.toml, the standard home for tool config. Now pytest gives 412 tests collected.
I also dropped pytest.ini from .gitignore. It takes precedence over pyproject.toml, so a
leftover existing local one silently overrides this. But the pyproject.toml is enough.
(Note: The machine-specific parts live in settings_local.py so this ini was never needed to gitignore.).
Also swapped the deprecated pytest.yield_fixture for pytest.fixture.
Ignore the first commit, that's the reset_stats fix from #725. It drops once that merges and I'll rebase this, if SHA changes.
https://github.com/GrandComicsDatabase/gcd-django/pull/726
(10 files)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()
@gemini-code-assist[bot] commented on this pull request.
This pull request replaces deprecated @pytest.yield_fixture decorators with @pytest.fixture across the test suite, migrates pytest configuration to pyproject.toml, and fixes a Django aggregation bug in scripts/reset_stats.py by appending .order_by() to clear default ordering. It also introduces a new test file test_reset_stats.py to verify stats resetting. The review feedback suggests removing a commented-out fixture in test_issue.py to clean up dead code, and making an assertion in test_reset_stats.py more specific to ensure correct records are created.
In apps/gcd/tests/test_issue.py:
> +#@pytest.fixture
#def re_issue_mocks(any_series):
#with mock.patch('%s.from_all_reprints' % ISSUE_PATH) as from_mock, \
#mock.patch('%s.to_all_reprints' % ISSUE_PATH) as to_mock:
This commented-out fixture appears to be dead code. To improve maintainability and reduce clutter, it's best to remove it. If this code is intended for future use, it should be tracked in an issue or moved to a separate branch. The same applies to the commented-out tests that use this fixture further down in the file.
In apps/gcd/tests/test_reset_stats.py:
> + alpha, _ = two_series + Series.objects.update(issue_count=7) + + reset_series_issue_count_cache() + + alpha.refresh_from_db() + assert alpha.issue_count == 0 + + +...@pytest.mark.django_db +def test_main_rebuilds_countstats(two_series): + CountStats.objects.all().delete() + + main() + + assert CountStats.objects.exists()
The assertion assert CountStats.objects.exists() is a good start, but it could be more specific to ensure that main() is behaving as expected. Since the two_series fixture sets up data for a specific country and language, you could assert that CountStats records are created for that country and language. This would make the test more robust against future changes.
- assert CountStats.objects.exists() + assert CountStats.objects.filter(country__code='zz').exists() + assert CountStats.objects.filter(language__code='zz').exists()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()