-
Notifications
You must be signed in to change notification settings - Fork 5
chore: Add tests for template tags #53
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
Conversation
Reviewer's GuideThis PR removes an unused Site import from test factories and introduces a comprehensive suite of tests for the djangocms_stories template tags, validating URL generation, media placeholder handling, edit-mode behavior, integrations, and loop usage. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The test_templatetags.py file is very large and could be split into separate modules per tag (namespace_url, media_plugins, absolute_url) to improve organization and maintainability.
- A lot of setup code (creating the request, toolbar, and context) is duplicated across tests—consider extracting common fixtures or helper functions to DRY up the tests.
- Many assertions use loose checks like startswith or in; strengthen these by comparing to expected URLs via Django’s reverse() or using regex to catch regressions more reliably.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test_templatetags.py file is very large and could be split into separate modules per tag (namespace_url, media_plugins, absolute_url) to improve organization and maintainability.
- A lot of setup code (creating the request, toolbar, and context) is duplicated across tests—consider extracting common fixtures or helper functions to DRY up the tests.
- Many assertions use loose checks like startswith or in; strengthen these by comparing to expected URLs via Django’s reverse() or using regex to catch regressions more reliably.
## Individual Comments
### Comment 1
<location> `tests/test_templatetags.py:103` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Don't import test modules. ([`dont-import-test-modules`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/dont-import-test-modules))
<details><summary>Explanation</summary>Don't import test modules.
Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
</details>
</issue_to_address>
### Comment 2
<location> `tests/test_templatetags.py:119` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Don't import test modules. ([`dont-import-test-modules`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/dont-import-test-modules))
<details><summary>Explanation</summary>Don't import test modules.
Tests should be self-contained and don't depend on each other.
If a helper function is used by multiple tests,
define it in a helper module,
instead of importing one test from the other.
</details>
</issue_to_address>
### Comment 3
<location> `tests/test_templatetags.py:291-292` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #53 +/- ##
==========================================
+ Coverage 84.20% 86.57% +2.36%
==========================================
Files 23 23
Lines 2070 2070
Branches 232 232
==========================================
+ Hits 1743 1792 +49
+ Misses 220 186 -34
+ Partials 107 92 -15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by Sourcery
Add a suite of tests for djangocms_stories template tags and clean up unused test factory code
Tests:
Chores: