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

Adapt to latest Theia version #9

Merged
merged 2 commits into from
Mar 14, 2024
Merged

Adapt to latest Theia version #9

merged 2 commits into from
Mar 14, 2024

Conversation

marcdumais-work
Copy link
Contributor

Following this recent Theia commit [1], one now has to use "TheiaAppLoader.load()", to load a Theia application in a theia playwright templace. Some code here needs to be updated.

Also, use the latest versions (as of now) for @theia/playwright and @playwright/test

[1]: "Basic playwright electron support (#12207)"
eclipse-theia/theia@487e92b

@marcdumais-work marcdumais-work marked this pull request as ready for review March 13, 2024 20:44
@marcdumais-work
Copy link
Contributor Author

I will also open a PR towards the main Theia repo to update the following documentation about using the new way of "loading" the app:

Copy link
Contributor

@planger planger left a comment

Choose a reason for hiding this comment

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

Thanks a lot @marcdumais-work, looks good to me and works fine! 👍

@marcdumais-work
Copy link
Contributor Author

Thanks for the quick review @planger !
Ok to merge?

Following this recent Theia commit [1], one now has to use "TheiaAppLoader.load()",
to load a Theia application in a theia playwright template. Some code here needs to
be updated.

Also, use the latest versions (as of now) for @theia/playwright and @playwright/test

[1]: "Basic playwright electron support (#12207)"
eclipse-theia/theia@487e92b

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
@marcdumais-work
Copy link
Contributor Author

BTW, we have started to make use of this template and the related test infrastructure from @theia/playwright: we integrated the template as a starting point for a UI test suite in our eclipse-cdt-cloud/theia-trace-extension repo:
eclipse-cdt-cloud/theia-trace-extension#1052

@marcdumais-work
Copy link
Contributor Author

marcdumais-work commented Mar 14, 2024

Update: I noticed there's a workflow in this repository, but CI was disabled because it had not run in a couple of months, I was able to re-enable it in the GitHub UI (Actions tab) and manually launch the tests for this PR. It failed:
https://github.com/eclipse-theia/theia-playwright-template/actions/runs/8281847489/job/22661382243

I think it's missing some OS-level dependencies, for Theia to build. I will have a look, and since I probably need to update the workflow, I may as well clean it up similar to eclipse-theia/theia#13491

update: no extra apt dependency needed, when I look at theia main repo workflows. They do install node-gyp globally - maybe that's what's needed.

@marcdumais-work marcdumais-work force-pushed the update-latest-theia branch 16 times, most recently from 1e5e2a7 to f2fcd2b Compare March 14, 2024 15:58
It turns-out that building Theia works with Python 3.11 (same version
used in Theia workflows) but not with version 3.12.2 that was pulled
before (when we were requesting 3.x). With the later version, node-gyp
would error-out while attempting to build the Theia native dependencies.

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
@marcdumais-work
Copy link
Contributor Author

It turns-out that the Theia build issue was apparently caused by a later version of Python being pulled. Now the workflow uses 3.11, like Theia workflows, and Theia builds.

@planger
Copy link
Contributor

planger commented Mar 14, 2024

Oh, thank you very much for re-enabling the CI and tracking down the CI issue!

@marcdumais-work marcdumais-work merged commit c8e26f6 into main Mar 14, 2024
2 checks passed
@marcdumais-work marcdumais-work deleted the update-latest-theia branch March 14, 2024 17:27
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.

2 participants