Skip to content

Refactor event tests#1961

Open
johannaengland wants to merge 10 commits into
Uninett:mainfrom
johannaengland:tests/refactor-event-tests
Open

Refactor event tests#1961
johannaengland wants to merge 10 commits into
Uninett:mainfrom
johannaengland:tests/refactor-event-tests

Conversation

@johannaengland

@johannaengland johannaengland commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Scope and purpose

Dependent on #1504. After I added tests to this file in that pull request I decided it is time to make this test easier to read and for new tests to be added if wanted.

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to Argus can be found in the
Development docs.

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation, including updates to the user manual if feature flow or UI is considerably changed
  • Linted/formatted the code with ruff and djLint, easiest by using pre-commit
  • The first line of the commit message continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See our how-to
  • If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done
  • If this results in changes in the UI: Added screenshots of the before and after
  • If this results in changes to the database model: Updated the ER diagram

@johannaengland johannaengland requested a review from a team June 18, 2026 10:48
@johannaengland johannaengland self-assigned this Jun 18, 2026
@johannaengland johannaengland added the tests Needs tests/affects test framework, incl. tox label Jun 18, 2026
@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.86%. Comparing base (423246f) to head (5952abf).
⚠️ Report is 22 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1961      +/-   ##
==========================================
+ Coverage   88.63%   88.86%   +0.23%     
==========================================
  Files         149      152       +3     
  Lines        7186     7283      +97     
==========================================
+ Hits         6369     6472     +103     
+ Misses        817      811       -6     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@johannaengland johannaengland force-pushed the tests/refactor-event-tests branch from feed7ed to eac9ee2 Compare June 29, 2026 09:20
@johannaengland johannaengland added the nonews No news fragment is necessary for this PR (e.g. refactoring, cleanups, workflow/development changes) label Jun 29, 2026

@lunkwill42 lunkwill42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall a good restructure, though a couple of small things appear to need fixing...

Also, there is some setUp() logic that seems duplicated between classes. In lack of pytest fixtures (sigh), perhaps some of these could/should be factored out to a mixin class (I don't consider this a blocker for the PR, though, it's just a style hint)

Comment thread tests/incident/test_event.py Outdated
Comment thread tests/incident/test_event.py
self.assertFalse(self.stateless_incident.stateful)
self.assertFalse(self.stateless_incident.open)

def test_when_posting_reopen_event_then_incident_does_not_change(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This appears to be a sibling of test_when_posting_close_event_for_stateless_incident_then_incident_does_not_change(), but makes no mention of _for_stateless_incident in its name..

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, because it's been moved to the class EventAPIStatelessIncidentTests, so we have the stateless in the class name already

@sonarqubecloud

sonarqubecloud Bot commented Jul 9, 2026

Copy link
Copy Markdown

@johannaengland johannaengland requested review from a team and lunkwill42 and removed request for a team July 9, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

nonews No news fragment is necessary for this PR (e.g. refactoring, cleanups, workflow/development changes) tests Needs tests/affects test framework, incl. tox

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants