Skip to content

Conversation

ceorourke
Copy link
Member

@ceorourke ceorourke commented Oct 3, 2025

While working on https://getsentry.atlassian.net/browse/ACI-482 I was not able to find a test that asserted the output of fetch_metric_issue_open_periods. Since I'll be changing the response I wanted a test to be able to compare against and this one only checked that it was called with the expected params.

This is a little futile since we'll be moving away from this but it's helpful as a starting point.

@ceorourke ceorourke requested a review from a team as a code owner October 3, 2025 23:19
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Oct 3, 2025
class FetchOpenPeriodsTest(TestCase):
@freeze_time(frozen_time)
@patch("sentry.incidents.charts.client.get")
@with_feature("organizations:incidents")
Copy link
Member Author

Choose a reason for hiding this comment

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

Without this we were actually hitting a 404 hitting the incidents endpoint, but the test didn't care to check

@ceorourke ceorourke requested a review from a team October 3, 2025 23:20
Copy link

codecov bot commented Oct 3, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #100936   +/-   ##
========================================
  Coverage   81.11%    81.12%           
========================================
  Files        8659      8659           
  Lines      384312    384312           
  Branches    24251     24251           
========================================
+ Hits       311746    311759   +13     
+ Misses      72222     72209   -13     
  Partials      344       344           

@ceorourke ceorourke marked this pull request as draft October 3, 2025 23:43
@ceorourke ceorourke force-pushed the ceorourke/incident-chart-test-coverage branch 2 times, most recently from 18aa429 to fbc260e Compare October 7, 2025 00:07
@ceorourke ceorourke force-pushed the ceorourke/incident-chart-test-coverage branch from fbc260e to f1f16e6 Compare October 7, 2025 00:08
time_period = incident_date_range(60, incident.date_started, incident.date_closed)

fetch_metric_issue_open_periods(self.organization, detector.id, time_period)
mock_client_get.assert_called_with(
Copy link
Member Author

Choose a reason for hiding this comment

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

I recognize that I'm changing what's being tested here, but I'm not sure the previous test was adding much value. I can keep it around and add this as an additional test if necessary.

@ceorourke ceorourke marked this pull request as ready for review October 7, 2025 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant