Skip to content

Conversation

@KeyJayY
Copy link
Collaborator

@KeyJayY KeyJayY commented Dec 9, 2025

This Pull Request introduces significant stability improvements to address flaky CI tests. While further optimizations may be required, this PR is being submitted now due to its increasing size. Additionally, it includes code refactoring to improve readability.

How Has This Been Tested:

Tests have been executed on local CI. They seem to work much better after changes.

How Has This Change Been Documented:

No documentation changes are required.

@vercel
Copy link

vercel bot commented Dec 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
radon-ide Ready Ready Preview Comment Dec 11, 2025 10:40am

if (!appWebsocket) {
reject(new Error("No websocket connection"));

if (!appWebsocket || appWebsocket.readyState !== 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!appWebsocket || appWebsocket.readyState !== 1) {
if (!appWebsocket || appWebsocket.readyState !== WebSocket.OPEN) {

reject(
new Error(
`Websocket not connected or not ready. State: ${
appWebsocket ? appWebsocket.readyState : "null"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT:
Isn't this the same as

Suggested change
appWebsocket ? appWebsocket.readyState : "null"
appWebsocket?.readyState

Comment on lines 61 to 68
try {
const msg = JSON.parse(message);
if (msg.id === id) {
clearTimeout(timer);
appWebsocket.off("message", handler);
resolve(msg);
}
} catch (e) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we really just ignore the errors from parsing message here? It would seem to me that it indicates a bug in the tests, and should be reported.

const fullPath = path.join(process.cwd(), "data", filename);
await quickInput.sendKeys(fullPath);
await this.driver.sleep(TIMEOUTS.SHORT);
// await quickInput.sendKeys(Key.ENTER);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the comment, or uncomment the line if it was removed by mistake

Suggested change
// await quickInput.sendKeys(Key.ENTER);

@KeyJayY KeyJayY merged commit 6ac31d6 into main Dec 11, 2025
8 checks passed
@KeyJayY KeyJayY deleted the @KeyJayY/testsReadme branch December 11, 2025 10:45
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.

3 participants