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

fix: Update codeUtil.ts to download vscode for linux-arm64 #1161

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

djelinek
Copy link
Collaborator

Before submitting your PR, please review the following checklist:

  • CONSIDER adding a new test if your PR resolves an issue.
  • DO keep pull requests small so they can be easily reviewed.
  • DO make sure tests pass.
  • DO make sure any public APIs changes are documented.
  • DO make sure not to introduce any compiler warnings.

Before merging the PR:

  • CHECK continuous integration of main branch is green.
  • CHECK pull request check job is green.
  • CHECK all pull request questions/requests are resolved.
  • WAIT till PR is approved by at least 1 committer.

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@djelinek
Copy link
Collaborator Author

based on #1090

Thank you for you initial PR @timimichr . I am adding this one to include this in next release ASAP.

@djelinek djelinek mentioned this pull request Feb 27, 2024
9 tasks
@djelinek djelinek marked this pull request as ready for review February 27, 2024 17:15
@djelinek djelinek merged commit 4e8ad5e into redhat-developer:main Feb 27, 2024
11 checks passed
@djelinek djelinek added the bug Something isn't working label Feb 27, 2024
@timimichr
Copy link
Contributor

Do you have any estimate on when it can be solved regarding the chromedrivers as well?

@djelinek
Copy link
Collaborator Author

Do you have any estimate on when it can be solved regarding the chromedrivers as well?

what exactly do you mean?

@timimichr
Copy link
Contributor

timimichr commented Feb 29, 2024

In the draft PR: https://github.com/redhat-developer/vscode-extension-tester/pull/1090/files
There were also an POC on how to fix this so that it downloads correct chromedrivers for linux arm, and you wrote a comment about it would take more work to support linux arm for chromedriver.

While this PR, that is now merged and in the latest release, makes the package download correct vscode for linux arm it still downloads x64 chromedrivers.

See logs:

Downloading` VS Code: 1.86.2 / stable
**Downloading VS Code from: https://update.code.visualstudio.com/1.86.2/linux-arm64/stable**
progress: 0/165 (0%)
progress: 137533176/137533176 (100%)
Downloaded VS Code into /home/circleci/project/test/test-resources/stable.tar.gz
Unpacking VS Code into /home/circleci/project/test/test-resources
Success!
**Downloading ChromeDriver 118.0.5993.70 from: https://edgedl.me.gvt1.com/edgedl/chrome/chrome-for-testing/118.0.5993.70/linux64/chromedriver-linux64.zip**
progress: 0/8269742 (0%)
progress: 8269742/8269742 (100%)
Unpacking ChromeDriver 118.0.5993.70 into /home/circleci/project/test/test-resources
Success!
Executing prepublish script 'npm run vscode:prepublish'...

Am only running this on a Linux Arm machine in a pipeline, which makes it a bit harder to debug for me. But I believe this is the reason that the extension later fails to run the vscode-browser.
Why I believe this is the cause is that I had the same experience before on mac arm, when this was not supported through the extension tester.

@djelinek
Copy link
Collaborator Author

yes right, thanks for reminding me.

It would require a lot more work because as you mentioned in PR comment near suggested solution there needs to be dynamic version set based on electron version used by VS Code version. and then used that one only for linux arm64 platform..

or do complete switch from Chrome for testing to Electron Chromedriver which would require even much more work currently

but you are right.. I have missed it won't work from scratch because it will download right version of VS Code for linux arm but not the correct platform version of chromedriver (which is not available at all currently at Chrome for testing side 🙁 )

I would ask you to open new issue directly for supporting linux arm64 driver and we can take a look further later.. if you would be interested to provide more complex fix for it maybe?

btw macos arm should be supported already for vscode and chromedriver

@djelinek
Copy link
Collaborator Author

opened tracker on side of chrome for testing regarding missing chromedriver binaries for Linux ARM - GoogleChromeLabs/chrome-for-testing#1

@timimichr
Copy link
Contributor

Continue the conversation here instead of opening an issue directly, so the issue can be properly formulated.

Since the chromedriver is not available on Chrome yet, would it be possible to add a function that allows the user to set url of the chromedriver download link themself.
That is, allowing user to override the url where the chrome driver gets downloaded from.

In the POC PR I open, I had a hard coded url that links to linux arm chromedrivers.
But I was not able to test if the drivers from this url worked all the way.
I tested this by extending the ExTester when used with the api setup and overriding some fields and functions.
It downloaded the desired chromedriver on setup.

But it seemed as if either codeUtil_1 or driverUtil_1 (IIRC) got re-initiated at some time during the execution time as well. And this caused the run not to find the chromedriver. This re-initialisation cause it to look at the "default" path again, because it was not re-initialized from the ExTester instance that I had created with my overrides.

But it might also be possible that even if it looked at the desired chromedriver (set by me) it would fail. I have to little knowledge of the different chrome drivers to know.
It might have worked when running on a local machine, but possible not when running in pipeline.

I hope this make sense.
And wondering if you think such method would be possible, and if it might solve this issue while waiting for Chrome to give a linux arm driver for testing.

@djelinek
Copy link
Collaborator Author

@timimichr ok I follow your thoughts. I think it should be possible but I fear it still would need kinda lot of work and basically imho I feel to make it easy maintainable we would need to rework current implementation because there are two options currently how to achieve desired

  1. switch from current chrome for testing driver to electron chromedriver
  2. use current chrome for testing driver and use electron chromedriver as backup option to try when chrome for testing is not available

but basically both needs to solve first the matching logic for electron versions which would ve desired VS Code built on..

another story would be to make it nice and easy by extracting managing driver logic to separate package which would be then possible to parametrise and more easily modify to these changes which then just reused for ExTester (which is btw plan anyway for some unseen future now unfortunately)... sooo long story short.. it is possible but it depends

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🚫 Bug] Download vscode and chromedriver on linux arm targets wrong version of both
2 participants