Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Remove webview serializer #878

Merged
merged 4 commits into from
Nov 18, 2020
Merged

Remove webview serializer #878

merged 4 commits into from
Nov 18, 2020

Conversation

vitaliy-guliy
Copy link
Contributor

Signed-off-by: Vitaliy Gulyy vgulyy@redhat.com

What does this PR do?

Disables storing state of Welcome page to prevent opening two Welcome pages.

It's a copy of PR #848, which was merged and then reverted.

Should be merged after merging necessary bugfix to upstream eclipse-theia/theia#8621

What issues does this PR fix or reference?

Happy Path Channel

HAPPY_PATH_CHANNEL=stable

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Oct 13, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:878
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:878

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

ℹ️ Use comment "[crw-ci-test]" to rerun happy path E2E test.

@ericwill
Copy link
Contributor

Just to clarify @vitaliy-guliy, this PR should be fine because the upstream fix is merged now. It failed last time because the upstream one wasn't merged, correct?

@vitaliy-guliy
Copy link
Contributor Author

vitaliy-guliy commented Oct 13, 2020

Just to clarify @vitaliy-guliy, this PR should be fine because the upstream fix is merged now. It failed last time because the upstream one wasn't merged, correct?

The upstream fix is not merged yet eclipse-theia/theia#8621

I think the reason why it failed before was that we didn't use the latest commit of theia. The theia version that we used had a bug in restoring of opened files and webviews. I just merged that PR before we switched to the head theia commit.

To say the truth, it's not very clear what was wrong with that PR as all checks and tests passed.

@ericwill ericwill mentioned this pull request Oct 22, 2020
29 tasks
@ericwill ericwill mentioned this pull request Nov 12, 2020
34 tasks
Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 17, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:878
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:878

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 17, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:878
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:878

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@dmytro-ndp
Copy link
Contributor

[crw-ci-test]

@che-bot
Copy link
Contributor

che-bot commented Nov 17, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:878
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:878

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@vitaliy-guliy
Copy link
Contributor Author

[crw-ci-test]

Copy link
Member

@azatsarynnyy azatsarynnyy left a comment

Choose a reason for hiding this comment

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

I've tested several cases:

  • w/o opened editors - Welcome page is opened
  • with an opened editor - Welcome page is not opened
  • Enable/disable welcome plugin user's preference is respected as well

Everything works as expected 👍
Big thanks for the upstream patch #848!

@che-bot
Copy link
Contributor

che-bot commented Nov 17, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:878
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:878

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@che-bot
Copy link
Contributor

che-bot commented Nov 17, 2020

❌ E2E Happy path tests failed ❗

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:878
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:878

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Nov 17, 2020

@vitaliy-guliy: Could you, please, take a look at Happy path tests failure #878 (comment) ?
Test failed on waiting of workspace readiness - there was no element with xpath = "//div[@id='theia-left-content-panel']//ul[@Class='p-TabBar-content']//li[@title[contains(.,'Explorer')] and contains(@id, 'shell-tab')])'

Here error message from theia container:

2020-11-17 16:38:15.021 root ERROR [hosted-plugin: 54] TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'CommandRegistryImpl'
    |     property 'commandsConverter' -> object with constructor 'CommandsConverter'
    --- property 'commands' closes the circle
    at JSON.stringify (<anonymous>)
    at Function.MessageFactory.request (/home/theia/node_modules/@theia/plugin-ext/lib/common/rpc-protocol.js:363:166)
    at RPCProtocolImpl.remoteCall (/home/theia/node_modules/@theia/plugin-ext/lib/common/rpc-protocol.js:190:46)
    at Proxy.target.<computed> (/home/theia/node_modules/@theia/plugin-ext/lib/common/rpc-protocol.js:164:38)
    at SourceControlImpl.set (/home/theia/node_modules/@theia/plugin-ext/lib/plugin/scm.js:332:24)
    at /default-theia-plugins/vscode-git/extension/dist/main.js:1:73426
    at /default-theia-plugins/vscode-git/extension/dist/main.js:1:4316
    at /home/theia/node_modules/@theia/core/lib/common/event.js:176:33
    at CallbackList.invoke (/home/theia/node_modules/@theia/core/lib/common/event.js:191:39)
    at Emitter.fire (/home/theia/node_modules/@theia/core/lib/common/event.js:318:29)
TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'CommandRegistryImpl'
    |     property 'commandsConverter' -> object with constructor 'CommandsConverter'
    --- property 'commands' closes the circle
    at JSON.stringify (<anonymous>)
    at Function.MessageFactory.request (/home/theia/node_modules/@theia/plugin-ext/lib/common/rpc-protocol.js:363:166)
    at RPCProtocolImpl.remoteCall (/home/theia/node_modules/@theia/plugin-ext/lib/common/rpc-protocol.js:190:46)
    at Proxy.target.<computed> (/home/theia/node_modules/@theia/plugin-ext/lib/common/rpc-protocol.js:164:38)
    at SourceControlImpl.set (/home/theia/node_modules/@theia/plugin-ext/lib/plugin/scm.js:332:24)
    at /default-theia-plugins/vscode-git/extension/dist/main.js:1:73426
    at /default-theia-plugins/vscode-git/extension/dist/main.js:1:4316
    at /home/theia/node_modules/@theia/core/lib/common/event.js:176:33
    at CallbackList.invoke (/home/theia/node_modules/@theia/core/lib/common/event.js:191:39)
    at Emitter.fire (/home/theia/node_modules/@theia/core/lib/common/event.js:318:29) 

@vitaliy-guliy
Copy link
Contributor Author

@vitaliy-guliy: Could you, please, take a look at Happy path tests failure #878 (comment) ?
Test failed on waiting of workspace readiness - there was no element with xpath = "//div[@id='theia-left-content-panel']//ul[@Class='p-TabBar-content']//li[@title[contains(.,'Explorer')] and contains(@id, 'shell-tab')])'

It's still unclear for me what's wrong, but it definitely does not depend on changes in this PR.
This PR only prevents appearing extra Welcome pages when refreshing Che-Theia.

Signed-off-by: Vitaliy Gulyy <vgulyy@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Nov 18, 2020

✅ E2E Happy path tests succeed 🎉

Try Che-Theia editor only Try Che-Theia with Java/maven example Try Che-Theia with NodeJs example

See Details

name link
che-theia docker.io/maxura/che-theia:878
che-theia-endpoint-runtime-binary docker.io/maxura/che-theia-endpoint-runtime-binary:878

Tested with Eclipse Che Single User on K8S (minikube v1.1.1)

  • Use comment "[crw-ci-test]" to rerun happy path E2E test.
  • Use comment "[crw-ci-test --rebuild]" to re-build the images and rerun happy path E2E test.

Copy link
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

Happy path tests have passed #878 (comment)

@vitaliy-guliy vitaliy-guliy merged commit 8157662 into master Nov 18, 2020
@vitaliy-guliy vitaliy-guliy deleted the serialize-webview branch November 18, 2020 15:59
vinokurig pushed a commit that referenced this pull request Apr 6, 2021
Change-Id: Icf244c3255edee69d2ac8093154acf860e92929c
Signed-off-by: nickboldt <nboldt@redhat.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants