-
Notifications
You must be signed in to change notification settings - Fork 111
Conversation
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
@benoitf I would be interested if you see any major problems with my approach. |
@tsmaeder I think you've done two steps in this PR (two birds, one stone) and usually when we mix some concerns it's hard to get it merge easily. There are linkless and 'generator-less' as well (BTW there is still a script to run: so maybe target first linkless (as for example an option in generator) so everyone can try and there is no such breakage on all existing jobs. and if committers don't like generator, we could remove it as well (another PR/step/issue) |
@benoitf I'm looking for a validation of the concept before I update the generator. I was able to come up with a PoC in 1-2 days: fixing the generator to work would take me longer (I'd have to dive into it first). |
@tsmaeder if it's introduced as a new flag I don't see any issue at all |
@tsmaeder do you have a devfile that would setup that POC ? |
@tsmaeder just rm -rf theia and che-theia if you have them, but the referenced devfile starts with no projects anyway. |
step 8. maybe having a volume for HOME would help not to link twice. |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
f85e4c2
to
cfce3d2
Compare
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
cfce3d2
to
7d24fc2
Compare
✅ E2E Happy path tests succeed 🎉 See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
assembly/package.json
Outdated
@@ -70,7 +70,7 @@ | |||
"scripts": { | |||
"prepare": "yarn run clean && yarn build", | |||
"clean": "theia clean && rimraf errorShots", | |||
"build": "theia build --mode development --config cdn/webpack.config.js --env.cdn=./cdn.json --env.monacopkg=@theia/[email protected] && yarn run override-vs-loader", | |||
"build": "NODE_OPTIONS='--max-old-space-size=4096' theia build --mode development --config cdn/webpack.config.js --env.cdn=./cdn.json --env.monacopkg=@theia/[email protected] && yarn run override-vs-loader", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would move it to .github/workflows/pr.yml
ENV variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've thought about this a bit more, and I think it's correct to set this only for the theia build: the size of memory needed depends on the task being executed, not the environment. Other tasks don't need that much max memory, and giving it to them can make them grab more than they need and take the memory away from other processes, thus reducing potential concurrency.
Codecov Report
@@ Coverage Diff @@
## master #955 +/- ##
==========================================
+ Coverage 29.45% 32.40% +2.95%
==========================================
Files 277 285 +8
Lines 9336 9683 +347
Branches 1380 1461 +81
==========================================
+ Hits 2750 3138 +388
+ Misses 6487 6447 -40
+ Partials 99 98 -1
Continue to review full report at Codecov.
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
ef8d15e
to
a18112e
Compare
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che Single User on K8S (minikube v1.1.1)
|
45e61e9
to
6bb4ad7
Compare
FYI I've an error on the link test:
|
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che on K8S (minikube v1.1.1)
Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe |
@benoitf I can't reproduce the test failure. How are you running the tests? |
@tsmaeder I think it's related to symlinks where theia is cloned |
So are you saying your link folder is itself in a symlinked location? |
@tsmaeder yes |
it's a bug in yarn but unfortunately it's not yet solved (yarnpkg/yarn#7028 and yarnpkg/yarn#1297) |
❌ E2E Happy path tests failed ❗ See Details
Tested with Eclipse Che on K8S (minikube v1.1.1)
Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe |
@tsmaeder could you please rebase/resolve the conflict ? so we can have PR checks running |
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder <[email protected]>
b61fda7
to
237e589
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
❌ E2E Happy path tests failed ❗ See Details
Test product:
Eclipse Che QE channel: https://mattermost.eclipse.org/eclipse/channels/eclipse-che-qe |
Introduce build process using yarn link Signed-off-by: Thomas Mäder <[email protected]>
Signed-off-by: Thomas Mäder [email protected]
What does this PR do?
This is a proof of concept of a build process for che-theia that does not involve linking source folders: linking source folders introduces some problems because tools that manipulate paths will either see /projects/theia as a parent of a che-theia package or not, depending on whether the tools uses resolved file system paths or just manipulates the path string.
The idea of the PR is to move the
assembly
folder as a yarn workspace folder inside the che-theia repo and to yarn link all @Theia packages at the level of the che-theia repo root.What issues does this PR fix or reference?
eclipse-che/che#16852
How to test this PR?
yarn
inside theia; wait for the build to finishlinktheia.sh
yarn
cd assembly && yarn run start --hostname=0.0.0.0 --port=3010 --plugins=local-dir:/projects/theia/plugins,local-dir:/projects/che-theia/plugins
linktheia.sh
script in the plugin host containercd /projects/che-theia/extensions/eclipse-che-theia-plugin-remote/lib/node && node plugin-remote.js
theia-dev-flow
endpointif you want to build against the latest Theia release (instead of against master), you can just remove the
@theia
folder from/projects/che-theia/node_modules
and rerunyarn
.PR Checklist
As the author of this Pull Request I made sure that:
Reviewers
Reviewers, please comment how you tested the PR when approving it.
Happy Path Channel
HAPPY_PATH_CHANNEL=next