Skip to content

Annotate tests based on staff.ga-rollout status#107368

Merged
michelletran-sentry merged 2 commits intomasterfrom
test_tests_move_to_staff
Feb 3, 2026
Merged

Annotate tests based on staff.ga-rollout status#107368
michelletran-sentry merged 2 commits intomasterfrom
test_tests_move_to_staff

Conversation

@michelletran-sentry
Copy link
Copy Markdown
Contributor

@michelletran-sentry michelletran-sentry commented Jan 30, 2026

My attempt to clean up the feature flag resulted in a lot of changed files (#106966) . I'm opening up smaller PRs to migrate the tests more slowly. Then end goal is to change as many of the tests' functionality to use the staff flag (to preserve test coverage), then tag the tests that I can just delete in the migration (specific superuser tests).

I'm starting with tests in the following directories:

  • tests/sentry/relocation
  • tests/sentry/users/api

I'll probably have followups to do the tagging more slowly.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Comment thread tests/sentry/users/api/endpoints/test_user_details.py
Comment thread tests/sentry/relocation/api/endpoints/test_index.py
Comment thread tests/sentry/users/api/endpoints/test_user_index.py
Copy link
Copy Markdown
Contributor

@cathteng cathteng left a comment

Choose a reason for hiding this comment

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

Looks like we can get rid of a lot of tests! I was mostly looking for cases we would delete

Comment thread tests/sentry/users/api/endpoints/test_user_details.py Outdated
Comment thread tests/sentry/relocation/api/endpoints/test_recover.py
Comment thread tests/sentry/relocation/api/endpoints/artifacts/test_details.py Outdated
Comment thread tests/sentry/relocation/api/endpoints/artifacts/test_details.py Outdated
Comment thread tests/sentry/relocation/api/endpoints/test_cancel.py Outdated
Comment thread tests/sentry/relocation/api/endpoints/test_details.py Outdated
Comment thread tests/sentry/relocation/api/endpoints/test_abort.py
Comment thread tests/sentry/relocation/api/endpoints/test_index.py Outdated
This is to prepare the option for removal
Add @override_options({"staff.ga-rollout": True/False}) decorators to
tests in tests/sentry/users/api/endpoints/ to ensure tests run with
appropriate feature flag configuration.
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

response = self.get_success_response(qs_params={"query": "is:superuser"})
assert len(response.data) == 1
assert response.data[0]["id"] == str(self.superuser.id)
assert len(response.data) == 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Renamed test doesn't verify meaningful functionality

Medium Severity

The test test_staff_query was renamed from test_superuser_query but still queries is:superuser and now asserts 0 results. The original test verified the is:superuser query filter worked correctly by finding 1 superuser. The new test doesn't verify any query functionality - it just confirms that querying for non-existent users returns empty results, which provides no meaningful test coverage.

Fix in Cursor Fix in Web

@michelletran-sentry michelletran-sentry merged commit 724230a into master Feb 3, 2026
69 checks passed
@michelletran-sentry michelletran-sentry deleted the test_tests_move_to_staff branch February 3, 2026 16:24
michelletran-sentry added a commit that referenced this pull request Feb 10, 2026
Similar to #107368, where I'm
trying to convert and mark tests that depends on superusers who should
be migrated to staff. This ensures all tests work with
staff.ga-rollout=True while maintaining test coverage during the
transition. Superuser tests in duplicate pairs can be easily identified
and removed later. This is part of the effort to remove the
staff.ga-rollout flag and migrate to staff-only mode.

Changes:
- Converted 5 superuser-only tests to staff in
test_sentry_app_details.py:
  - test_staff_can_set_publish_request_inprogress_status
  - test_staff_delete_unpublished_app
  - test_staff_delete_unpublished_app_with_installs
  - test_staff_cannot_delete_published_app
  - test_staff_cannot_delete_partner_apps
- Converted 1 superuser-only test to staff in test_sentry_apps_stats.py:
  - test_per_page
- Updated DeleteSentryAppDetailsTest setUp to use staff user by default:
  - Creates staff user as org member in setUp
  - Logs in as staff instead of superuser
  - Simplifies individual test methods (no repeated membership creation)
- Added decorators to ALL tests (47 total):
  - False decorator on superuser tests in duplicate pairs
  - True decorator on all other tests (staff and general tests)

Co-authored-by: Claude <noreply@anthropic.com>
jaydgoss pushed a commit that referenced this pull request Feb 12, 2026
My attempt to clean up the feature flag resulted in a lot of changed
files (#106966) . I'm opening up
smaller PRs to migrate the tests more slowly. Then end goal is to change
as many of the tests' functionality to use the staff flag (to preserve
test coverage), then tag the tests that I can just delete in the
migration (specific `superuser` tests).

I'm starting with tests in the following directories:

- `tests/sentry/relocation`
- `tests/sentry/users/api`

I'll probably have followups to do the tagging more slowly.

### Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated
in the State of Delaware in 2015 as Functional Software, Inc. and is
gonna need some rights from me in order to utilize my contributions in
this here PR. So here's the deal: I retain all rights, title and
interest in and to my contributions, and by keeping this boilerplate
intact I confirm that Sentry can use, modify, copy, and redistribute my
contributions, under Sentry's choice of terms.
jaydgoss pushed a commit that referenced this pull request Feb 12, 2026
Similar to #107368, where I'm
trying to convert and mark tests that depends on superusers who should
be migrated to staff. This ensures all tests work with
staff.ga-rollout=True while maintaining test coverage during the
transition. Superuser tests in duplicate pairs can be easily identified
and removed later. This is part of the effort to remove the
staff.ga-rollout flag and migrate to staff-only mode.

Changes:
- Converted 5 superuser-only tests to staff in
test_sentry_app_details.py:
  - test_staff_can_set_publish_request_inprogress_status
  - test_staff_delete_unpublished_app
  - test_staff_delete_unpublished_app_with_installs
  - test_staff_cannot_delete_published_app
  - test_staff_cannot_delete_partner_apps
- Converted 1 superuser-only test to staff in test_sentry_apps_stats.py:
  - test_per_page
- Updated DeleteSentryAppDetailsTest setUp to use staff user by default:
  - Creates staff user as org member in setUp
  - Logs in as staff instead of superuser
  - Simplifies individual test methods (no repeated membership creation)
- Added decorators to ALL tests (47 total):
  - False decorator on superuser tests in duplicate pairs
  - True decorator on all other tests (staff and general tests)

Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 19, 2026
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