-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix failing to start shell being hidden from user #5626
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
Changes from 7 commits
ed6f522
87dc14c
dba4f59
8136e23
f111450
c9f1594
2de3068
d293554
988307e
071ba8a
ea603c7
78b05ba
aa41117
19466f1
d1acca4
f2d19e7
2a96eb8
862cd2a
3c9791e
c340256
f522d23
37955f0
b86bc6d
4c1d076
c6a9d56
e49b0f6
d173e45
b96e115
6ce833c
0dabbb6
a82a120
e90a181
9c75199
97dfe4d
e8bc887
58cbf5f
8761aa1
8fd732b
e9e248e
71ad8e1
3a7ac10
c11d8c6
0088add
6321b10
9683a7c
a271c83
3b72137
19a8691
f6b5214
78614af
56703cd
a1d2fa0
3fbbf02
8b18e58
d9a7a48
ea49929
0e1b2c0
12bd506
2f43d24
14f50a3
6d0e527
d22af76
3492741
6dd926a
cfad2c0
186d7f9
9adddaf
e88743d
4885461
120a5f7
91cd2c8
7971755
c658a0b
9b7e77d
4815510
3471397
e77411a
d1eb149
6880d52
d2ee54f
8704067
9539c1c
5f30b81
4edfa40
29a7e2a
51b8c91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,8 +118,7 @@ | |
| "<rootDir>/packages" | ||
| ], | ||
| "setupFiles": [ | ||
| "<rootDir>/src/jest.setup.ts", | ||
| "jest-canvas-mock" | ||
| "<rootDir>/src/jest.setup.ts" | ||
| ], | ||
| "globalSetup": "<rootDir>/src/jest.timezone.ts", | ||
| "setupFilesAfterEnv": [ | ||
|
|
@@ -265,7 +264,7 @@ | |
| "immer": "^9.0.17", | ||
| "joi": "^17.7.0", | ||
| "js-yaml": "^4.1.0", | ||
| "jsdom": "^16.7.0", | ||
| "jsdom": "^20.0.1", | ||
| "lodash": "^4.17.15", | ||
| "marked": "^4.2.5", | ||
| "md5-file": "^5.0.0", | ||
|
|
@@ -301,8 +300,7 @@ | |
| "win-ca": "^3.5.0", | ||
| "winston": "^3.8.2", | ||
| "winston-transport-browserconsole": "^1.0.5", | ||
| "ws": "^8.12.0", | ||
| "xterm-link-provider": "^1.3.1" | ||
| "ws": "^8.12.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@async-fn/jest": "1.6.4", | ||
|
|
@@ -335,6 +333,7 @@ | |
| "@types/html-webpack-plugin": "^3.2.6", | ||
| "@types/http-proxy": "^1.17.9", | ||
| "@types/jest": "^28.1.6", | ||
| "@types/jest-image-snapshot": "^5.1.0", | ||
| "@types/js-yaml": "^4.0.5", | ||
| "@types/jsdom": "^16.2.14", | ||
| "@types/lodash": "^4.14.191", | ||
|
|
@@ -372,8 +371,10 @@ | |
| "@types/webpack-node-externals": "^2.5.3", | ||
| "@typescript-eslint/eslint-plugin": "^5.48.1", | ||
| "@typescript-eslint/parser": "^5.48.1", | ||
| "@types/ws": "^8.5.4", | ||
| "adr": "^1.4.3", | ||
| "ansi_up": "^5.1.0", | ||
| "canvas": "^2.10.1", | ||
| "chalk": "^4.1.2", | ||
| "chart.js": "^2.9.4", | ||
| "circular-dependency-plugin": "^5.2.2", | ||
|
|
@@ -403,8 +404,8 @@ | |
| "ignore-loader": "^0.1.2", | ||
| "include-media": "^1.4.9", | ||
| "jest": "^28.1.3", | ||
| "jest-canvas-mock": "^2.3.1", | ||
| "jest-environment-jsdom": "^28.1.3", | ||
| "jest-image-snapshot": "^5.2.0", | ||
| "jest-mock-extended": "^2.0.9", | ||
| "make-plural": "^6.2.2", | ||
| "memfs": "^3.4.12", | ||
|
|
@@ -447,28 +448,16 @@ | |
| "webpack-cli": "^4.9.2", | ||
| "webpack-dev-server": "^4.11.1", | ||
| "webpack-node-externals": "^3.0.0", | ||
| "xterm": "^4.19.0", | ||
| "xterm-addon-fit": "^0.5.0" | ||
| "xterm": "^5.0.0", | ||
| "xterm-addon-fit": "^0.5.0", | ||
|
||
| "xterm-addon-search": "^0.10.0", | ||
| "xterm-addon-web-links": "^0.7.0", | ||
| "xterm-addon-webgl": "^0.13.0", | ||
| "xterm-link-provider": "^1.3.1" | ||
| }, | ||
| "peerDependencies": { | ||
| "@types/byline": "^4.2.33", | ||
| "@types/chart.js": "^2.9.36", | ||
| "@types/color": "^3.0.3", | ||
| "@types/crypto-js": "^3.1.47", | ||
| "@types/lodash": "^4.14.191", | ||
| "@types/proper-lockfile": "^4.1.2", | ||
| "@types/react-dom": "^17.0.16", | ||
| "@types/react-router-dom": "^5.3.3", | ||
| "@types/react-virtualized-auto-sizer": "^1.0.1", | ||
| "@types/react-window": "^1.8.5", | ||
| "@types/request-promise-native": "^1.0.18", | ||
| "@types/tar": "^6.1.3", | ||
| "@types/tcp-port-used": "^1.0.1", | ||
| "@types/url-parse": "^1.4.8", | ||
| "@types/uuid": "^8.3.4", | ||
| "monaco-editor": "^0.29.1", | ||
| "react-select": "^5.7.0", | ||
| "typed-emitter": "^1.4.0", | ||
| "xterm-addon-fit": "^0.5.0" | ||
| "typed-emitter": "^1.4.0" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,10 @@ describe("app-paths", () => { | |
| }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| builder.quit(); | ||
| }); | ||
|
|
||
|
||
| describe("normally", () => { | ||
| let windowDi: DiContainer; | ||
| let mainDi: DiContainer; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /** | ||
| * Copyright (c) OpenLens Authors. All rights reserved. | ||
| * Licensed under MIT License. See LICENSE in root directory for more information. | ||
| */ | ||
|
|
||
| import { getGlobalOverride } from "../test-utils/get-global-override"; | ||
| import createReadFileStreamInjectable from "./create-read-file-stream.injectable"; | ||
|
|
||
| export default getGlobalOverride(createReadFileStreamInjectable, () => () => { | ||
| throw new Error("tried to create read stream for a file without override"); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,15 +11,15 @@ import { getApplicationBuilder } from "../../../renderer/components/test-utils/g | |
| import type { LensWindow } from "../../../main/start-main-application/lens-window/application-window/create-lens-window.injectable"; | ||
| import type { MessageChannel } from "./message-channel-listener-injection-token"; | ||
| import { messageChannelListenerInjectionToken } from "./message-channel-listener-injection-token"; | ||
| import type { RequestFromChannel } from "./request-from-channel-injection-token"; | ||
| import { requestFromChannelInjectionToken } from "./request-from-channel-injection-token"; | ||
| import type { RequestChannel } from "./request-channel-listener-injection-token"; | ||
| import type { RequestChannel } from "./request-channel"; | ||
| import type { AsyncFnMock } from "@async-fn/jest"; | ||
| import asyncFn from "@async-fn/jest"; | ||
| import { getPromiseStatus } from "../../test-utils/get-promise-status"; | ||
| import { runInAction } from "mobx"; | ||
| import type { RequestChannelHandler } from "../../../main/utils/channel/channel-listeners/listener-tokens"; | ||
| import { getRequestChannelListenerInjectable } from "../../../main/utils/channel/channel-listeners/listener-tokens"; | ||
| import type { RequestFromChannel } from "../../../renderer/utils/channel/request-from-channel.injectable"; | ||
| import requestFromChannelInjectable from "../../../renderer/utils/channel/request-from-channel.injectable"; | ||
|
Comment on lines
+21
to
+22
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't be removing the injection token.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important thing is that channel is something that can be called from anywhere and it doesn't know who answers. That being said, the "request" or "message" doesn't even have to go over IPC, the listener and caller can be located in same environment. (It doesn't work like this yet due YAGNI but conceptually that's what Channel is.)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The contract isn't the same. A request channel being requested on has no meaning on
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No it cannot, where is that possible? |
||
|
|
||
| type TestMessageChannel = MessageChannel<string>; | ||
| type TestRequestChannel = RequestChannel<string, string>; | ||
|
|
@@ -60,6 +60,10 @@ describe("channel", () => { | |
| messageToChannel = mainDi.inject(sendMessageToChannelInjectionToken); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| builder.quit(); | ||
| }); | ||
|
|
||
| describe("given window is started", () => { | ||
| let someWindowFake: LensWindow; | ||
|
|
||
|
|
@@ -103,10 +107,10 @@ describe("channel", () => { | |
| describe("messaging from renderer to main, given listener for channel in a main and application has started", () => { | ||
| let messageListenerInMainMock: jest.Mock; | ||
| let messageToChannel: SendMessageToChannel; | ||
| let builder: ApplicationBuilder; | ||
|
|
||
| beforeEach(async () => { | ||
| const applicationBuilder = getApplicationBuilder(); | ||
|
|
||
| builder = getApplicationBuilder(); | ||
| messageListenerInMainMock = jest.fn(); | ||
|
|
||
| const testChannelListenerInMainInjectable = getInjectable({ | ||
|
|
@@ -120,19 +124,23 @@ describe("channel", () => { | |
| injectionToken: messageChannelListenerInjectionToken, | ||
| }); | ||
|
|
||
| applicationBuilder.beforeApplicationStart((mainDi) => { | ||
| builder.beforeApplicationStart((mainDi) => { | ||
| runInAction(() => { | ||
| mainDi.register(testChannelListenerInMainInjectable); | ||
| }); | ||
| }); | ||
|
|
||
| await applicationBuilder.render(); | ||
| await builder.render(); | ||
|
|
||
| const windowDi = applicationBuilder.applicationWindow.only.di; | ||
| const windowDi = builder.applicationWindow.only.di; | ||
|
|
||
| messageToChannel = windowDi.inject(sendMessageToChannelInjectionToken); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| builder.quit(); | ||
| }); | ||
|
|
||
| it("when sending message, triggers listener in main", () => { | ||
| messageToChannel(testMessageChannel, "some-message"); | ||
|
|
||
|
|
@@ -143,30 +151,32 @@ describe("channel", () => { | |
| describe("requesting from main in renderer, given listener for channel in a main and application has started", () => { | ||
| let requestListenerInMainMock: AsyncFnMock<RequestChannelHandler<TestRequestChannel>>; | ||
| let requestFromChannel: RequestFromChannel; | ||
| let builder: ApplicationBuilder; | ||
|
|
||
| beforeEach(async () => { | ||
| const applicationBuilder = getApplicationBuilder(); | ||
|
|
||
| builder = getApplicationBuilder(); | ||
| requestListenerInMainMock = asyncFn(); | ||
|
|
||
| const testChannelListenerInMainInjectable = getRequestChannelListenerInjectable({ | ||
| channel: testRequestChannel, | ||
| handler: () => requestListenerInMainMock, | ||
| }); | ||
|
|
||
| applicationBuilder.beforeApplicationStart((mainDi) => { | ||
| builder.beforeApplicationStart((mainDi) => { | ||
| runInAction(() => { | ||
| mainDi.register(testChannelListenerInMainInjectable); | ||
| }); | ||
| }); | ||
|
|
||
| await applicationBuilder.render(); | ||
| await builder.render(); | ||
|
|
||
| const windowDi = applicationBuilder.applicationWindow.only.di; | ||
| const windowDi = builder.applicationWindow.only.di; | ||
|
|
||
| requestFromChannel = windowDi.inject( | ||
| requestFromChannelInjectionToken, | ||
| ); | ||
| requestFromChannel = windowDi.inject(requestFromChannelInjectable); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| builder.quit(); | ||
| }); | ||
|
|
||
| describe("when requesting from channel", () => { | ||
|
|
@@ -196,28 +206,38 @@ describe("channel", () => { | |
| }); | ||
| }); | ||
|
|
||
| it("when registering multiple handlers for the same channel, throws", async () => { | ||
| const applicationBuilder = getApplicationBuilder(); | ||
| describe("when registering multiple handlers for the same channel", () => { | ||
| let builder: ApplicationBuilder; | ||
|
|
||
| const testChannelListenerInMainInjectable = getRequestChannelListenerInjectable({ | ||
| channel: testRequestChannel, | ||
| handler: () => () => "some-value", | ||
| }); | ||
| const testChannelListenerInMain2Injectable = getRequestChannelListenerInjectable({ | ||
| channel: testRequestChannel, | ||
| handler: () => () => "some-other-value", | ||
| }); | ||
| beforeEach(() => { | ||
| builder = getApplicationBuilder(); | ||
|
|
||
| const testChannelListenerInMainInjectable = getRequestChannelListenerInjectable({ | ||
| channel: testRequestChannel, | ||
| handler: () => () => "some-value", | ||
| }); | ||
| const testChannelListenerInMain2Injectable = getRequestChannelListenerInjectable({ | ||
| channel: testRequestChannel, | ||
| handler: () => () => "some-other-value", | ||
| }); | ||
|
|
||
| testChannelListenerInMain2Injectable.id += "2"; | ||
| testChannelListenerInMain2Injectable.id += "2"; | ||
|
|
||
| applicationBuilder.beforeApplicationStart((mainDi) => { | ||
| runInAction(() => { | ||
| mainDi.register(testChannelListenerInMainInjectable); | ||
| mainDi.register(testChannelListenerInMain2Injectable); | ||
| builder.beforeApplicationStart((mainDi) => { | ||
| runInAction(() => { | ||
| mainDi.register(testChannelListenerInMainInjectable); | ||
| mainDi.register(testChannelListenerInMain2Injectable); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
| await expect(applicationBuilder.render()).rejects.toThrow('Tried to register a multiple channel handlers for "some-request-channel-id", only one handler is supported for a request channel.'); | ||
| afterEach(() => { | ||
| builder.quit(); | ||
| }); | ||
|
|
||
| it("throws on startup", async () => { | ||
| await expect(builder.render()).rejects.toThrow('Tried to register a multiple channel handlers for "some-request-channel-id", only one handler is supported for a request channel.'); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
This file was deleted.
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.
This commit removes the last usage for earlier introduce image-snapshot dependency. Could you remove it?
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.
Sure also