Skip to content

Conversation

@wedamija
Copy link
Member

@wedamija wedamija commented Jan 8, 2025

This removes tests that generate snapshots and fail whenever we add in new models. I don't think these tests have caught anything useful recently, and they cause a lot of noise and frustration when creating models.

After this, we'll still need to run bin/generate-model-dependency-fixtures, but not the snapshot tests

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 8, 2025
@codecov
Copy link

codecov bot commented Jan 8, 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   #83140      +/-   ##
==========================================
- Coverage   87.60%   87.59%   -0.02%     
==========================================
  Files        9461     9461              
  Lines      536643   536554      -89     
  Branches    21097    21097              
==========================================
- Hits       470142   469988     -154     
- Misses      66142    66207      +65     
  Partials      359      359              

Copy link
Contributor

@saponifi3d saponifi3d left a comment

Choose a reason for hiding this comment

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

🙏 may want to have someone with more context also review, but lgtm. thanks for making migrations easier.

(test failures are just to remove the import for the SANITIZATION_TESTED set though, maybe more updates in https://github.com/getsentry/sentry/blob/master/tests/sentry/backup/test_coverage.py ?)

Copy link
Member

@hubertdeng123 hubertdeng123 left a comment

Choose a reason for hiding this comment

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

Given that the owner of these tests is now no longer with us, and since they haven't caught anything useful this seems fine to do

Copy link
Contributor

@asottile-sentry asottile-sentry left a comment

Choose a reason for hiding this comment

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

This removes tests that generate snapshots and fail whenever we add in new models. I don't think these tests have caught anything useful recently, and they cause a lot of noise and frustration when creating models.

After this, we'll still need to run `bin/generate-model-dependency-fixtures`, but not the snapshot tests
@wedamija wedamija force-pushed the danf/migrations-remove-snapshot-tests branch from 0042148 to 8fd78f9 Compare January 9, 2025 18:16
@wedamija wedamija enabled auto-merge (squash) January 9, 2025 18:18
@wedamija wedamija merged commit 5a9c9a5 into master Jan 9, 2025
44 checks passed
@wedamija wedamija deleted the danf/migrations-remove-snapshot-tests branch January 9, 2025 18:44
andrewshie-sentry pushed a commit that referenced this pull request Jan 22, 2025
This removes tests that generate snapshots and fail whenever we add in
new models. I don't think these tests have caught anything useful
recently, and they cause a lot of noise and frustration when creating
models.

After this, we'll still need to run
`bin/generate-model-dependency-fixtures`, but not the snapshot tests

<!-- Describe your PR here. -->
@github-actions github-actions bot locked and limited conversation to collaborators Jan 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants