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.

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

chore: Improve test coverage #395

Merged
merged 22 commits into from
Apr 24, 2024

Conversation

mgax
Copy link
Contributor

@mgax mgax commented Mar 28, 2024

Fix #378.

  • Refactor the existing tests to use wagtail-factories and clean them up a bit.
  • Fix an annoying issue where running tests locally would pollute the shared memcache and break the generation of page URLs for the development instance.
  • Remove some unused or overly complicated code (e.g. the OrderedSet implementation).
  • Add tests for previously untested areas of the code (e.g. bibliography, snippets, cache purging, the megamenu).
  • Fix a couple of subtle bugs:
    • The blog redirect to IESG statements would format the date_to URL parameter incorrectly.
    • The megamenu URLs for direct children of the menu item's page would be generated incorrectly.
  • Mark some bits of the code as # pragma: no cover because they are impractical / not useful to test.

@mgax
Copy link
Contributor Author

mgax commented Mar 29, 2024

I've pushed an update to cover IAB-specific pages too.

Copy link

@SharmaineLim SharmaineLim left a comment

Choose a reason for hiding this comment

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

Hey Alex, just have a few comments, and something to consider for the tests.

ietf/utils/signal_handlers.py Outdated Show resolved Hide resolved
ietf/snippets/factories.py Show resolved Hide resolved
ietf/home/models.py Show resolved Hide resolved
ietf/bibliography/models.py Show resolved Hide resolved
ietf/announcements/tests.py Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.89%. Comparing base (4ad3143) to head (1da2d8b).
Report is 7 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #395       +/-   ##
===========================================
+ Coverage   82.72%   95.89%   +13.16%     
===========================================
  Files         124      143       +19     
  Lines        2599     3117      +518     
===========================================
+ Hits         2150     2989      +839     
+ Misses        449      128      -321     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@SharmaineLim SharmaineLim left a comment

Choose a reason for hiding this comment

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

The tests are looking good to me, I just want more documentation. :)

Also flagging a small concern about coupling the BE and FE a bit more tightly with the tests.

ietf/announcements/tests.py Show resolved Hide resolved
ietf/bibliography/tests.py Show resolved Hide resolved
ietf/announcements/tests.py Show resolved Hide resolved
ietf/utils/tests/test_mega_menu.py Show resolved Hide resolved
@mgax
Copy link
Contributor Author

mgax commented Apr 3, 2024

@SharmaineLim I've added more docstrings and comments. I've also updated CONTRIBUTING.md to explain the use of pytest.

@mgax mgax mentioned this pull request Apr 3, 2024
@mgax mgax marked this pull request as ready for review April 4, 2024 10:23
@kesara kesara changed the base branch from main to feat/modern-footer April 23, 2024 20:38
@kesara kesara merged commit a19c6b7 into ietf-tools:feat/modern-footer Apr 24, 2024
1 check passed
@mgax mgax deleted the improve-test-coverage branch April 24, 2024 09:23
@kesara kesara mentioned this pull request May 26, 2024
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.

Improve test coverage
4 participants