fix typo #2
Merged
fix typo #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
originally from @amorisot at sourcegraph/sourcegraph#54398 Co-authored-by: Adrien Morisot <adrien.morisot@gmail.com>
philipp-spiess
added a commit
that referenced
this pull request
Aug 21, 2023
philipp-spiess
added a commit
that referenced
this pull request
Aug 21, 2023
…#773) ## Test plan This builds now for real 😓 ``` α vscode (main)✗ pnpm vsce package --no-dependencies Executing prepublish script 'npm run vscode:prepublish'... > cody-ai@0.8.0 vscode:prepublish > pnpm -s run build dist/extension.node.js 6.0mb⚠️ dist/extension.node.js.map 9.6mb ⚡ Done in 149ms vite v4.4.3 building for production... ✓ 438 modules transformed. ../dist/webviews/index.html 0.78 kB ../dist/webviews/codicon-2d58c995.ttf 70.78 kB ../dist/webviews/index-024934e3.css 57.90 kB ../dist/webviews/index.js 1,759.42 kB │ map: 12,839.48 kB ✓ built in 2.18s DONE Packaged: /Users/philipp/dev/cody/vscode/cody-ai-0.8.0.vsix (196 files, 27.05MB) INFO The latest version of @vscode/vsce is 2.20.1 and you have 2.19.0. Update it now: npm install -g @vscode/vsce ``` <!-- Required. See https://docs.sourcegraph.com/dev/background-information/testing_principles. -->
DanTup
added a commit
that referenced
this pull request
Dec 2, 2023
This fixes two flakes in recent tests I added: 1. VS Code toast notifications obscuring the chatbox so the test could not click them. The failure looked like: ``` retrying click action, attempt #2 waiting 20ms waiting for element to be visible, enabled and stable element is visible, enabled and stable scrolling into view if needed done scrolling <div title="Git (Extension)" class="notification-list…>Source: Git (Extension)</div> from <div class="notifications-toasts visible">…</div> subtree intercepts pointer events ``` This was fixed by turning on Do Not Disturb mode (something already done in another test, so I extracted a function and reused it). 2. Triggering rate limits in the second test sometimes not working. This one was confusing. It appears that in e2e tests we stop the server, we just call `server.close()` which only prevents new connections. The server continues to run for any open connections. This could result in the next test from reusing some existing connections to the first server and creating some new connections to its own new server. For tests that modify state in the mock server (eg. to trigger rate limits) this would cause failures. Adding unique IDs to the server and some logging resulted in this: ``` ✓ 1 chat.test.ts:8:5 › shows appropriate rate limit message for free users (4.4s) (Server: 4): STARTING SERVER... (Server: 4): chat rate limited (free) (Server: 4): handling chat responses true, false ✘ 2 chat.test.ts:24:5 › shows appropriate rate limit message for pro users (9.3s) (Server: 440): STARTING SERVER... (Server: 4): chat rate limited (pro) <------------ This is the wrong server! (Server: 440): handling chat responses false, false ✓ 3 chat.test.ts:24:5 › shows appropriate rate limit message for pro users (retry #1) (4.4s) (Server: 278): STARTING SERVER... (Server: 278): chat rate limited (pro) (Server: 278): handling chat responses true, true - 4 inline-chat.test.ts:16:6 › start a fixup job from inline chat with valid auth ``` The fix is to ensure all existing connections to the server are closed when we close the server.
DanTup
added a commit
that referenced
this pull request
Dec 2, 2023
This fixes two flakes in recent tests I added: 1. VS Code toast notifications obscuring the chatbox so the test could not click them. The failure looked like: ``` retrying click action, attempt #2 waiting 20ms waiting for element to be visible, enabled and stable element is visible, enabled and stable scrolling into view if needed done scrolling <div title="Git (Extension)" class="notification-list…>Source: Git (Extension)</div> from <div class="notifications-toasts visible">…</div> subtree intercepts pointer events ``` This was fixed by turning on Do Not Disturb mode (something already done in another test, so I extracted a function and reused it). 2. Triggering rate limits in the second test sometimes not working. This one was confusing. It appears that in e2e tests we stop the server, we just call `server.close()` which only prevents new connections. The server continues to run for any open connections. This could result in the next test from reusing some existing connections to the first server and creating some new connections to its own new server. For tests that modify state in the mock server (eg. to trigger rate limits) this would cause failures. Adding unique IDs to the server and some logging resulted in this:  The fix is to ensure all existing connections to the server are closed when we close the server (by collecting them as they open and calling `destroy()` on them). ## Test plan These are fixes to the test, green bots with no failures/retries indicate it works.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
originally from @amorisot at sourcegraph/sourcegraph#54398