Skip to content

fix: Fix breadcrumb timestamp casting and its tests#3546

Merged
sentrivana merged 27 commits intogetsentry:masterfrom
BYK:byk/tests/timezones
Sep 23, 2024
Merged

fix: Fix breadcrumb timestamp casting and its tests#3546
sentrivana merged 27 commits intogetsentry:masterfrom
BYK:byk/tests/timezones

Conversation

@BYK
Copy link
Member

@BYK BYK commented Sep 18, 2024

These tests were failing for me locally as the timestamps were without tzinfo and all were assumed UTC whereas my local timezone is BST at the moment.

This patch fixes the tests along with faulty/incomplete breadcrumb timestamp parsing logic on py3.7 and py3.8.

These tests were failing for me locally as the timestamps were without `tzinfo` and all were assumed UTC whereas my local timezone is BST at the moment. This patch fixes them by forcing things to have the UTC timezone.
@codecov
Copy link

codecov bot commented Sep 18, 2024

Codecov Report

Attention: Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.49%. Comparing base (25ab10c) to head (9099d97).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/scope.py 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3546   +/-   ##
=======================================
  Coverage   84.49%   84.49%           
=======================================
  Files         133      133           
  Lines       13844    13855   +11     
  Branches     2927     2930    +3     
=======================================
+ Hits        11697    11707   +10     
- Misses       1422     1423    +1     
  Partials      725      725           
Files with missing lines Coverage Δ
sentry_sdk/utils.py 86.44% <100.00%> (+0.17%) ⬆️
sentry_sdk/scope.py 87.91% <0.00%> (-0.14%) ⬇️

... and 2 files with indirect coverage changes

@antonpirker
Copy link
Contributor

Thanks @BYK for the PR. (we had a pr for that: #3537) but yours is nicer, so I will close the other one. :-)

@BYK
Copy link
Member Author

BYK commented Sep 19, 2024

@antonpirker ah, sorry 😅 was working on another patch and these failed. Didn't think of looking for an existing PR.

@BYK BYK requested a review from sentrivana September 19, 2024 15:22
Co-authored-by: Ivana Kellyer <ivana.kellyer@sentry.io>
@BYK BYK changed the title tests: Fix breadcrumb ordering tests fix: Fix breadcrumb timestamp casting and its tests Sep 21, 2024
@sentrivana sentrivana merged commit 26b86a5 into getsentry:master Sep 23, 2024
@BYK BYK deleted the byk/tests/timezones branch September 23, 2024 10:55
@szokeasaurusrex
Copy link
Member

One of the test_datetime_from_isoformat cases fails on a non-UTC system. Opening issue

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.

4 participants