Skip to content

Conversation

@timfish
Copy link
Collaborator

@timfish timfish commented Jan 24, 2024

Closes #10278

This integration incorrectly used setup which then caused issues when the core code was changed. It should have been using setupOnce.

Unfortunately, setupOnce causes issues with the unit tests since it then relies on getClient to get the client instance for the client options. I've tried using jest to mock getClient but this doesn't work which is likely due to installedIntegrations.

Because we already have extensive integration tests for LocalVariables I have removed the unit tests because they are becoming a maintenance burden and actually test less than the integration tests.

After v8, I plan to move Local variables lookup to the worker thread at which point we can revisit how these might be better tested in unit tests.

@timfish timfish marked this pull request as ready for review January 24, 2024 12:54
@AbhiPrasad AbhiPrasad merged commit d471dcf into getsentry:develop Jan 24, 2024
@timfish timfish deleted the fix/localvariables-setup-once branch January 24, 2024 14:16
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.

Local variables issue: The inspector session is already connected

2 participants