-
Notifications
You must be signed in to change notification settings - Fork 28
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
fix(uninstall): Only clear credentials if the Cody app was uninstalled #2485
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the previous iteration of this caused a regression, let's add a test for this.
@dominiccooney Is this something that would be easy to test with the existing integration tests? |
@jamesmcnamara I would think about it this way:
|
@@ -12,13 +12,18 @@ import com.sourcegraph.cody.agent.protocol.BillingProduct | |||
import com.sourcegraph.cody.agent.protocol.TelemetryEventParameters | |||
import com.sourcegraph.cody.config.CodyAuthenticationManager | |||
import com.sourcegraph.cody.telemetry.TelemetryV2 | |||
import com.sourcegraph.config.ConfigUtil | |||
import java.util.concurrent.TimeUnit | |||
|
|||
class UninstallListener : StartupActivity { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For unit test we can use |
@dominiccooney @pkukielka I've followed your advice and added a unit test using |
@dominiccooney @pkukielka Friendly Monday morning bump. I've added a test for the uninstaller. |
Take two of CODY-1043 that was reverted in #2484.
The issue is that uninstall events are triggered when any plugin is uninstalled and not just when the plugin that registered the handler is uninstalled. This checks that the plugin being uninstalled is in fact Cody before running the handler.
Test plan
I have manually tested that when uninstalling other extensions one branch of the
if
runs, and when uninstalling Cody the other half does.As discussed in #2434 it would be non-trivial to test this behavior because we will need to drive the installation and uninstallation of the plugin from an automated test driver, which will likely require a much more custom test kit.