Fix flaky completion test and clean up console #1864
Conversation
| @@ -184,7 +184,7 @@ async function doGetInlineCompletions(params: InlineCompletionsParams): Promise< | |||
| } | |||
|
|
|||
| // Do not trigger when the last character is a closing symbol | |||
| if (triggerKind !== TriggerKind.Manual && /[)\]}]$/.test(currentLinePrefix.trim())) { | |||
| if (triggerKind !== TriggerKind.Manual && /[);\]}]$/.test(currentLinePrefix.trim())) { | |||
There was a problem hiding this comment.
I wanted to sneak this in anyways; If we have a line like this:
foo.bar();█we don't want to trigger a completion regardless of wether we print a semi or not.
| if (process.env.CI === undefined) { | ||
| return | ||
| } |
There was a problem hiding this comment.
@akalia25 is it okay if this runs only on our CI servers? Otherwise this makes local development a PITA
There was a problem hiding this comment.
Yes, of course! I appreciate you checking 🙇
| @@ -123,9 +123,7 @@ export async function run<T>(around: () => Promise<T>): Promise<T> { | |||
| } | |||
| }) | |||
|
|
|||
| const server = app.listen(SERVER_PORT, () => { | |||
| console.log(`Mock server listening on port ${SERVER_PORT}`) | |||
There was a problem hiding this comment.
Not helpful. Tests should only log something if something fails IMO
|
@philipp-spiess I hadn't gotten to landing it yet, but I have some changes in #1535 (and #1541) to fix some flakes here too. Not sure if you'd seen that - does this PR supersede that, or might we need both? |
|
@DanTup Ahh I’m so sorry, I have completely overlooked your PR. I think with your approach works too to fix the flake. It's a bit bound to the completion provider (Anthropic in this case) because we parse the prompt but otherwise no big deal. I think we can land both fixes with the |
;(similar to)]})bliss!
Test plan