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

ref(escalating_issues): Easier to write and read tests #48102

Merged
merged 13 commits into from
May 1, 2023

Conversation

armenzg
Copy link
Member

@armenzg armenzg commented Apr 27, 2023

This improves the readability of the tests.

Some of the tests were time sensitive and the results would change depending on how close it would execute to midnight.

This uses freeze gun in order to have a tight control on the time for the tests.

This also includes some refactoring.
Before the tests would intermittently fail depending on the time of execution.

Freeze gun helps making the tests resilient.
@codecov
Copy link

codecov bot commented Apr 27, 2023

Codecov Report

Merging #48102 (9f41a43) into master (8ddc186) will increase coverage by 3.15%.
The diff coverage is n/a.

❗ Current head 9f41a43 differs from pull request most recent head 03c1bc4. Consider uploading reports for the commit 03c1bc4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #48102      +/-   ##
==========================================
+ Coverage   77.73%   80.88%   +3.15%     
==========================================
  Files        4745     4769      +24     
  Lines      201433   201640     +207     
  Branches    11542    11548       +6     
==========================================
+ Hits       156577   163095    +6518     
+ Misses      44601    38290    -6311     
  Partials      255      255              

see 438 files with indirect coverage changes

@@ -33,27 +33,28 @@ def setUp(self) -> None:
def _load_event_for_group(
self,
project_id: Optional[int] = None,
minutes_ago: int = 1,
count: int = 1,
hours_ago: int = 0,
Copy link
Member Author

Choose a reason for hiding this comment

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

Using hours_ago lines up more closely with the hourly bucket we use in my Snuba query.

@armenzg armenzg changed the title fix(escalating_issues): Make test not time sensitive ref(escalating_issues): Easier to write and read tests Apr 28, 2023
@armenzg armenzg self-assigned this Apr 28, 2023
@armenzg armenzg added this to the Escalating Issues V1 milestone Apr 28, 2023
@armenzg armenzg requested a review from jangjodi April 28, 2023 16:20
@armenzg armenzg marked this pull request as ready for review April 28, 2023 16:20
tests/sentry/issues/test_escalating.py Outdated Show resolved Hide resolved
group.substatus = GroupSubStatus.UNTIL_ESCALATING
group.save()

def assert_is_escalating(self, group: Group) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

this is great addition!

@armenzg armenzg merged commit ae631a8 into master May 1, 2023
@armenzg armenzg deleted the armenzg/escalating/intermittent-test branch May 1, 2023 12:18
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants