Un-register extension between steps to prevent cascading test failures #5227
Conversation
…ting all registered extensions. - Update ensureEditorServicesIsConnected test method to un-register the extension if its already registered to prevent cascading test failures.
There was a problem hiding this comment.
Pull Request Overview
This PR prevents cascading test failures by ensuring that any previously registered extension is unregistered before a new registration occurs.
- Adds a new getRegisteredExtensions method to the extension client.
- Updates the test to loop over registered extensions and unregister any that match the target extension ID.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/utils.ts | Updated test logic to unregister a pre-registered extension. |
| src/features/ExternalApi.ts | Added getRegisteredExtensions to expose registered extensions. |
| src/extension.ts | Extended the client wrapper to include the new getRegisteredExtensions method. |
Comments suppressed due to low confidence (1)
test/utils.ts:85
- The variable name '_name' is ambiguous. Consider renaming it to 'uuid' or 'extensionKey' to better convey its purpose.
for (const [
| if (externalExtension.id === extensionId) { | ||
| extension.unregisterExternalExtension(_name); | ||
| } | ||
| }; |
There was a problem hiding this comment.
[nitpick] There is an unnecessary semicolon after the for loop block; removing it would improve code readability.
| }; | |
| } |
|
@tsmarvin is there a particular issue this is fixing in the external API you are seeing? |
|
Sort of. I added this so that it was easier for me to debug the actual root cause issue I was having. I added it pretty early on in my testing so I'm not 100% confident on why I was seeing the cascading registration errors, but i suspect the PSES internal host not starting left the test waiting for the connection and it skipped over the de-registration on timeout. If you don't think it's necessary I won't be offended, just figured id submit the PR as it helped me personally when debugging. It seemed like it was the expected state for the start of several tests related to the method I added it to. |
2 participants
PR Summary
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready