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(test): Add TestManager struct for uniform test setup #2252

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

szokeasaurusrex
Copy link
Member

@szokeasaurusrex szokeasaurusrex commented Nov 15, 2024

Add TestManager to manage setting up Mocks and Trycmd tests. The TestManager's API makes it easier to achieve uniform setup of our integration tests. This will also make bumping to new Mockito (see #2223) easier, since the new Mockito API will require manually setting up a mock server which needs to stay alive for the duration of the test. Having a TestManager will abstract away the management of this server, so the calling code does not need to worry about this.

All tests have been migrated to the new TestManager API with this change, as we are also removing the old API. For the most part, all tests should be verifying the same things as they were before this change. However, there are two intentional behavior changes to the tests with the new API:

  • In the old API, the default register_test function injected the SENTRY_AUTH_TOKEN environment variable. To run a test without an auth token, there was a separate register_test_without_token function which did not inject this environment variable. The new API's default register_trycmd_test function does not inject the SENTRY_AUTH_TOKEN environment variable. To get the environment variable, you need to call with_default_token method on the TrycmdTestManager. This change adds the with_default_token call only to those tests which would fail without the SENTRY_AUTH_TOKEN environment variable, meaning that several tests where the token was previously present no longer have it, since they apparently don't need the token.
  • In some of our tests (particularly sourcemap uploads), we used to assert only the mocks generated by common_upload_endpoints, even when other mocks were also present. Now, the TestManager exposes an assert_mock_endpoints function which asserts all the mock endpoints on the manager. This change required a change to some of the additional mocks. One mock's path was made more specific (since it was called with a more specific path), and another mock was deleted since it was never called.

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/test-manager branch 2 times, most recently from 5f9b20b to 6c15b5b Compare November 15, 2024 10:35
@szokeasaurusrex szokeasaurusrex marked this pull request as ready for review November 15, 2024 10:35
Copy link
Contributor

@loewenheim loewenheim left a comment

Choose a reason for hiding this comment

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

On the whole this looks good, just one criticism really.

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/test-manager branch 3 times, most recently from b04f1bf to aec3cbe Compare November 15, 2024 12:48
Add `TestManager` to manage setting up Mocks and Trycmd tests. This will make bumping to new Mockito #2223 easier
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/test-manager branch from aec3cbe to b9d50ff Compare November 15, 2024 12:51
@szokeasaurusrex szokeasaurusrex merged commit b9d50ff into master Nov 15, 2024
14 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/test-manager branch November 15, 2024 12:55
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.

2 participants