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

Upgrade and adapt to new version of libraries in ogre-tools #7241

Merged
merged 3 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
72 changes: 36 additions & 36 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"test:unit": "jest --testPathIgnorePatterns integration",
"test:watch": "func() { jest ${1} --watch --testPathIgnorePatterns integration; }; func",
"lint": "PROD=true eslint --ext js,ts,tsx --max-warnings=0 .",
"lint:fix": "npm run lint --fix"
"lint:fix": "npm run lint -- --fix"
},
"config": {
"k8sProxyVersion": "0.3.0",
Expand Down Expand Up @@ -130,11 +130,11 @@
"@k8slens/node-fetch": "^6.4.0-beta.13",
"@kubernetes/client-node": "^0.18.1",
"@material-ui/styles": "^4.11.5",
"@ogre-tools/fp": "^12.0.1",
"@ogre-tools/injectable": "^12.0.1",
"@ogre-tools/injectable-extension-for-auto-registration": "^12.0.1",
"@ogre-tools/injectable-extension-for-mobx": "^12.0.1",
"@ogre-tools/injectable-react": "^12.0.1",
"@ogre-tools/fp": "^15.1.1",
"@ogre-tools/injectable": "^15.1.1",
"@ogre-tools/injectable-extension-for-auto-registration": "^15.1.1",
"@ogre-tools/injectable-extension-for-mobx": "^15.1.1",
"@ogre-tools/injectable-react": "^15.1.1",
"@sentry/electron": "^3.0.8",
"@sentry/integrations": "^6.19.3",
"@side/jest-runtime": "^1.1.0",
Expand Down
20 changes: 17 additions & 3 deletions packages/core/src/common/__tests__/cluster-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ describe("cluster-store", () => {
di.override(kubectlBinaryNameInjectable, () => "kubectl");
di.override(kubectlDownloadingNormalizedArchInjectable, () => "amd64");
di.override(normalizedPlatformInjectable, () => "darwin");
createCluster = di.inject(createClusterInjectionToken);
getCustomKubeConfigFilePath = di.inject(getCustomKubeConfigFilePathInjectable);

writeJsonSync = di.inject(writeJsonSyncInjectable);
writeFileSync = di.inject(writeFileSyncInjectable);
writeBufferSync = di.inject(writeBufferSyncInjectable);
Expand All @@ -85,6 +84,9 @@ describe("cluster-store", () => {

describe("empty config", () => {
beforeEach(async () => {
createCluster = di.inject(createClusterInjectionToken);
getCustomKubeConfigFilePath = di.inject(getCustomKubeConfigFilePathInjectable);

writeJsonSync("/some-directory-for-user-data/lens-cluster-store.json", {});
clusterStore = di.inject(clusterStoreInjectable);
clusterStore.load();
Expand Down Expand Up @@ -198,6 +200,10 @@ describe("cluster-store", () => {
},
],
});

createCluster = di.inject(createClusterInjectionToken);
getCustomKubeConfigFilePath = di.inject(getCustomKubeConfigFilePathInjectable);

clusterStore = di.inject(clusterStoreInjectable);
clusterStore.load();
});
Expand Down Expand Up @@ -249,6 +255,10 @@ describe("cluster-store", () => {
},
],
});

createCluster = di.inject(createClusterInjectionToken);
getCustomKubeConfigFilePath = di.inject(getCustomKubeConfigFilePathInjectable);

clusterStore = di.inject(clusterStoreInjectable);
clusterStore.load();
});
Expand All @@ -262,6 +272,11 @@ describe("cluster-store", () => {

describe("pre 3.6.0-beta.1 config with an existing cluster", () => {
beforeEach(() => {
di.override(storeMigrationVersionInjectable, () => "3.6.0");
Copy link
Contributor Author

@jansav jansav Feb 27, 2023

Choose a reason for hiding this comment

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

override has to happen before inject.


createCluster = di.inject(createClusterInjectionToken);
getCustomKubeConfigFilePath = di.inject(getCustomKubeConfigFilePathInjectable);

writeJsonSync("/some-directory-for-user-data/lens-cluster-store.json", {
__internal__: {
migrations: {
Expand All @@ -281,7 +296,6 @@ describe("cluster-store", () => {
});
writeBufferSync("/some-directory-for-user-data/icon_path", testDataIcon);

di.override(storeMigrationVersionInjectable, () => "3.6.0");

clusterStore = di.inject(clusterStoreInjectable);
clusterStore.load();
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/common/__tests__/user-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ describe("user store tests", () => {
get: () => "latest" as const,
init: async () => {},
}));

await di.inject(defaultUpdateChannelInjectable).init();

userStore = di.inject(userStoreInjectable);
});

describe("for an empty config", () => {
Expand All @@ -42,6 +42,8 @@ describe("user store tests", () => {
writeJsonSync("/some-directory-for-user-data/lens-user-store.json", {});
writeJsonSync("/some-directory-for-user-data/kube_config", {});

userStore = di.inject(userStoreInjectable);

userStore.load();
});

Expand Down Expand Up @@ -90,6 +92,8 @@ describe("user store tests", () => {

di.override(storeMigrationVersionInjectable, () => "10.0.0");

userStore = di.inject(userStoreInjectable);
Comment on lines 93 to +95
Copy link
Contributor Author

Choose a reason for hiding this comment

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

override has to happen before inject


userStore.load();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/

import type { DiContainer } from "@ogre-tools/injectable";
import createClusterInjectable from "../../../main/create-cluster/create-cluster.injectable";
import clusterFrameContextForNamespacedResourcesInjectable from "../../../renderer/cluster-frame-context/for-namespaced-resources.injectable";
import hostedClusterInjectable from "../../../renderer/cluster-frame-context/hosted-cluster.injectable";
import { getDiForUnitTesting } from "../../../renderer/getDiForUnitTesting";
Expand All @@ -18,6 +17,7 @@ import { KubeApi } from "../kube-api";
import { KubeObject } from "../kube-object";
import { KubeObjectStore } from "../kube-object.store";
import maybeKubeApiInjectable from "../maybe-kube-api.injectable";
import { createClusterInjectionToken } from "../../cluster/create-cluster-injection-token";

// eslint-disable-next-line no-restricted-imports
import { KubeApi as ExternalKubeApi } from "../../../extensions/common-api/k8s-api";
Expand All @@ -43,7 +43,7 @@ describe("ApiManager", () => {
di.override(directoryForKubeConfigsInjectable, () => "/some-kube-configs");
di.override(storesAndApisCanBeCreatedInjectable, () => true);

const createCluster = di.inject(createClusterInjectable);
const createCluster = di.inject(createClusterInjectionToken);
Copy link
Contributor Author

@jansav jansav Feb 27, 2023

Choose a reason for hiding this comment

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

Reference equality is important here. Previously it worked because the implementation for createClusterInjectionToken in main and renderer had same ID.


di.override(hostedClusterInjectable, () => createCluster({
contextName: "some-context-name",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ import setupAutoRegistrationInjectable from "../../../renderer/before-frame-star
import { createMockResponseFromString } from "../../../test-utils/mock-responses";
import storesAndApisCanBeCreatedInjectable from "../../../renderer/stores-apis-can-be-created.injectable";
import directoryForUserDataInjectable from "../../app-paths/directory-for-user-data/directory-for-user-data.injectable";
import createClusterInjectable from "../../../main/create-cluster/create-cluster.injectable";
import hostedClusterInjectable from "../../../renderer/cluster-frame-context/hosted-cluster.injectable";
import directoryForKubeConfigsInjectable from "../../app-paths/directory-for-kube-configs/directory-for-kube-configs.injectable";
import apiManagerInjectable from "../api-manager/manager.injectable";
import type { DiContainer } from "@ogre-tools/injectable";
import ingressApiInjectable from "../endpoints/ingress.api.injectable";
import loggerInjectable from "../../logger.injectable";
import maybeKubeApiInjectable from "../maybe-kube-api.injectable";
import { createClusterInjectionToken } from "../../cluster/create-cluster-injection-token";

describe("KubeApi", () => {
let fetchMock: AsyncFnMock<Fetch>;
Expand All @@ -39,7 +39,7 @@ describe("KubeApi", () => {
di.override(directoryForKubeConfigsInjectable, () => "/some-kube-configs");
di.override(storesAndApisCanBeCreatedInjectable, () => true);

const createCluster = di.inject(createClusterInjectable);
const createCluster = di.inject(createClusterInjectionToken);

di.override(hostedClusterInjectable, () => createCluster({
contextName: "some-context-name",
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/common/k8s-api/__tests__/kube-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import setupAutoRegistrationInjectable from "../../../renderer/before-frame-star
import { createMockResponseFromStream, createMockResponseFromString } from "../../../test-utils/mock-responses";
import storesAndApisCanBeCreatedInjectable from "../../../renderer/stores-apis-can-be-created.injectable";
import directoryForUserDataInjectable from "../../app-paths/directory-for-user-data/directory-for-user-data.injectable";
import createClusterInjectable from "../../../main/create-cluster/create-cluster.injectable";
import hostedClusterInjectable from "../../../renderer/cluster-frame-context/hosted-cluster.injectable";
import directoryForKubeConfigsInjectable from "../../app-paths/directory-for-kube-configs/directory-for-kube-configs.injectable";
import apiKubeInjectable from "../../../renderer/k8s/api-kube.injectable";
Expand All @@ -36,6 +35,7 @@ import namespaceApiInjectable from "../endpoints/namespace.api.injectable";
// NOTE: this is fine because we are testing something that only exported
// eslint-disable-next-line no-restricted-imports
import { PodsApi } from "../../../extensions/common-api/k8s-api";
import { createClusterInjectionToken } from "../../cluster/create-cluster-injection-token";

describe("createKubeApiForRemoteCluster", () => {
let createKubeApiForRemoteCluster: CreateKubeApiForRemoteCluster;
Expand All @@ -48,7 +48,7 @@ describe("createKubeApiForRemoteCluster", () => {
di.override(directoryForKubeConfigsInjectable, () => "/some-kube-configs");
di.override(storesAndApisCanBeCreatedInjectable, () => true);

const createCluster = di.inject(createClusterInjectable);
const createCluster = di.inject(createClusterInjectionToken);

di.override(hostedClusterInjectable, () => createCluster({
contextName: "some-context-name",
Expand Down Expand Up @@ -154,7 +154,7 @@ describe("KubeApi", () => {
fetchMock = asyncFn();
di.override(fetchInjectable, () => fetchMock);

const createCluster = di.inject(createClusterInjectable);
const createCluster = di.inject(createClusterInjectionToken);
const createKubeJsonApi = di.inject(createKubeJsonApiInjectable);

di.override(hostedClusterInjectable, () => createCluster({
Expand Down
36 changes: 25 additions & 11 deletions packages/core/src/common/utils/channel/channel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ 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 {
getRequestChannelListenerInjectable,
requestChannelListenerInjectionToken,
} from "../../../main/utils/channel/channel-listeners/listener-tokens";

type TestMessageChannel = MessageChannel<string>;
type TestRequestChannel = RequestChannel<string, string>;
Expand Down Expand Up @@ -199,21 +202,32 @@ describe("channel", () => {
it("when registering multiple handlers for the same channel, throws", async () => {
const applicationBuilder = getApplicationBuilder();

const testChannelListenerInMainInjectable = getRequestChannelListenerInjectable({
channel: testRequestChannel,
handler: () => () => "some-value",
});
const testChannelListenerInMain2Injectable = getRequestChannelListenerInjectable({
channel: testRequestChannel,
handler: () => () => "some-other-value",
const someChannelListenerInjectable = getInjectable({
id: "some-channel-listener",

instantiate: () => ({
channel: testRequestChannel,
handler: () => () => "irrelevant",
}),

injectionToken: requestChannelListenerInjectionToken,
});

testChannelListenerInMain2Injectable.id += "2";
const someOtherChannelListenerInjectable = getInjectable({
id: "some-other-channel-listener",

instantiate: () => ({
channel: testRequestChannel,
handler: () => () => "irrelevant",
}),

injectionToken: requestChannelListenerInjectionToken,
});

Comment on lines +205 to 226
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mutating injectable ID is not possible and shouldn't be advertised.

applicationBuilder.beforeApplicationStart((mainDi) => {
runInAction(() => {
mainDi.register(testChannelListenerInMainInjectable);
mainDi.register(testChannelListenerInMain2Injectable);
mainDi.register(someChannelListenerInjectable);
mainDi.register(someOtherChannelListenerInjectable);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const withOrphanPromiseInjectable = getInjectable({
toBeDecorated,
withErrorLoggingFor(() => "Orphan promise rejection encountered"),
withErrorSuppression,
);
) as ((...args: any[]) => any);

decorated(...args);
};
Expand Down