Skip to content
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

Update Code to 1.81.1 #6385

Closed
wants to merge 4 commits into from
Closed

Update Code to 1.81.1 #6385

wants to merge 4 commits into from

Conversation

benz0li
Copy link
Contributor

@benz0li benz0li commented Aug 11, 2023

@benz0li benz0li requested a review from a team as a code owner August 11, 2023 07:36
@benz0li
Copy link
Contributor Author

benz0li commented Aug 11, 2023

Increase of build timeout is not required. It took something over 30 mins on my machine.

ℹ️ The pipeline for #6369 failed due to an e2e test error.

@benz0li
Copy link
Contributor Author

benz0li commented Aug 11, 2023

4.17.0-rc.1 291682e1c0ef545e89945741f7f05953bd83e9d9 with Code 1.81.1 is deployed at https://coder.jupyter.b-data.ch; Image R (base:test-devtools-docker).

Functionality [modified by patches] tested and found to work:

  • base-path
  • cli-window-open
  • local-storage
  • marketplace
  • proxy-url
  • service-worker
  • webview

Jupyter Notebooks also work fine:

@code-asher
Copy link
Member

code-asher commented Aug 11, 2023

Weird, I am not sure why the e2e tests are failing. I re-ran them a few times but it seems like code-server is not loading in time, and I am seeing similar issues locally, but the really weird thing is that it only happens if I have the browser dev tools open. Maybe they added some kind of debug tool and it runs with dev tools open or in the e2e tests for some reason and is hanging.

I will be able to investigate more next week.

@benz0li
Copy link
Contributor Author

benz0li commented Aug 14, 2023

I will be able to investigate more next week.

Thank you. It would be great to have a release by the end of this week.

@benz0li
Copy link
Contributor Author

benz0li commented Aug 18, 2023

Ping @code-asher

@code-asher
Copy link
Member

code-asher commented Aug 18, 2023

I have been trying to reproduce the hang but I cannot seem to get it to happen again.

I updated the callback path to account for the base path.

In addition to the tests above, I also tested display languages and the GitHub token injection (since some code around secrets changed).

I get a lot (~23) of "throttler is disposed" errors when closing the browser but it does not seem to cause any issues so I think we can ignore it. There is an open issue for it: microsoft/vscode#188929

Assuming CI passes I think we will be good to go.

@code-asher
Copy link
Member

e2e tests still timing out unfortunately.

I think the test failures are actually because Node modules are not
being installed so the page never loads, and for some reason npm install
is not exiting with non-zero when that happens.
pipefail would be ideal here but not sure how wide the support is yet.
@code-asher
Copy link
Member

Sorted the test hang, but now there seems to be some syntax issue preventing WebKit browsers from working.

@maeryo
Copy link

maeryo commented Aug 19, 2023

I tested by inserting "ls -alh /home/runner/work/code-server/code-server/lib/vscode/" into the "test-e2e.sh" file.
Below is a capture based on the current PR.
image

And the image below is the result of adding "with" to "actions/checkout@v3" in "build.yaml".
image
image

Regrettably, this isn't a solution. An error occurs that's very similar to what you all are experiencing.
image

Looking at the git history, not including the value "submodules: true" when calling "checkout@v3" has been done for a long time, so I don't think it's due to that. However, since the two results are different, I hope it serves as a clue to solve the problem.

I sincerely hope the issue will be resolved soon.
thank you!

@code-asher
Copy link
Member

code-asher commented Aug 19, 2023

Thank you for experimenting with it.

I ran the CI build in Safari and the syntax error seems to be coming from some static { } blocks. I guess Safari does not like these.

They are coming from here:
https://github.com/microsoft/vscode/blob/6c3e3dba23e8fadc360aed75ce363ba185c49794/src/vs/base/common/color.ts#L475-L482

It does not seem like this file has changed recently, so I wonder if the compilation target was changed and now it is outputting JavaScript that is not quite compatible with Safari.

@code-asher
Copy link
Member

code-asher commented Aug 19, 2023

Oh, to clarify the submodule, the e2e step downloads the built package from CI and uses that for the tests, so there is no need to populate lib/vscode.

@code-asher
Copy link
Member

code-asher commented Aug 19, 2023

In 1.81.1 static readonly white = new Color(new RGBA(255, 255, 255, 1)) gets transpiled into:

static {
  this.white = new I(new E(255, 255, 255, 1))
}

In the previous version we got:

I.white = new I(new E(255, 255, 255, 1))

I get the same error on vscode.dev so it appears to be an upstream issue.

@benz0li
Copy link
Contributor Author

benz0li commented Aug 20, 2023

I get the same error on vscode.dev so it appears to be an upstream issue.

No worries. We can also skip this version. If you feel the same, please close this issue pull request.

I only prefer versions that work absolutely fine for all browsers.

@benz0li
Copy link
Contributor Author

benz0li commented Aug 27, 2023

Let us focus on the next release, then: microsoft/vscode#189778

@benz0li benz0li closed this Aug 27, 2023
@code-asher
Copy link
Member

Sorry all, I was out last week so I was not able to get back to this. I think skipping makes a lot of sense, we will need to see if we can get it patched upstream in the meantime if it has not already been.

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