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 UI deps #3163

Merged
merged 12 commits into from Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from 10 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
33 changes: 11 additions & 22 deletions dashboard/package.json
Expand Up @@ -10,24 +10,6 @@
"@clr/ui": "^5.4.1",
"@improbable-eng/grpc-web": "^0.14.0",
"@paciolan/remote-component": "^2.11.0",
"@types/diff": "^5.0.1",
"@types/google-protobuf": "^3.15.3",
"@types/js-yaml": "^4.0.2",
"@types/json-schema": "^7.0.8",
"@types/jsonwebtoken": "^8.5.4",
"@types/lodash": "^4.14.171",
"@types/moxios": "^0.4.12",
"@types/pako": "^1.0.2",
"@types/qs": "^6.9.7",
"@types/react": "^17.0.15",
"@types/react-copy-to-clipboard": "^5.0.1",
"@types/react-jsonschema-form": "^1.7.6",
"@types/react-router-dom": "^5.1.8",
"@types/react-router-hash-link": "^2.4.1",
"@types/react-tabs": "^2.3.3",
"@types/react-transition-group": "^4.4.2",
"@types/semver": "^7.3.8",
"@types/ws": "^7.4.7",
Comment on lines -13 to -30
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to dev deps. Also, I removed all of them and just installed those ones Typescript was complaining (this way we reduce the types dependencies (as many of them are including also the definition of the types)

"ajv": "^8.6.2",
"axios": "^0.21.1",
"connected-react-router": "^6.9.1",
Expand All @@ -37,6 +19,7 @@
"google-protobuf": "^3.17.3",
"history": "^4.10.1",
"immutable": "^4.0.0-rc.14",
"isomorphic-form-data": "^2.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

So it sounds from @dlaloue-vmware 's comments that this dependency is required by swagger-ui-react but wasn't declared as a dependency by them? Or something similar: swagger-api/swagger-ui#7436

Hah, although they mention there that: "On Thursday 29.7.2021 there will a new release of swagger-ui-react that will fix this issue automatically by updating to it.". Not sure if we wait, but if not, let's at least include a #TODO to remove it refering to the swagger-ui issue ?

Copy link
Contributor Author

@antgamdia antgamdia Jul 30, 2021

Choose a reason for hiding this comment

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

While writing this message, they have just bumped up the version: https://github.com/swagger-api/swagger-ui/releases/tag/v3.51.2, so I have updated it and removed the unnecessary dep straight away. Thanks for the pointer!

"js-yaml": "^4.1.0",
"json-schema": "^0.3.0",
"jsonwebtoken": "^8.5.1",
Expand All @@ -59,7 +42,6 @@
"react-markdown": "^6.0.2",
"react-minimal-pie-chart": "^8.2.0",
"react-redux": "^7.2.4",
"react-router": "^5.2.0",
"react-router-dom": "^5.2.0",
"react-router-hash-link": "^2.4.3",
"react-switch": "^6.0.0",
Expand Down Expand Up @@ -119,14 +101,21 @@
"@formatjs/cli": "^4.2.29",
"@types/enzyme": "^3.10.9",
"@types/enzyme-adapter-react-16": "^1.0.6",
"@types/google-protobuf": "^3.15.3",
"@types/jest": "^26.0.24",
"@types/node": "^16.4.6",
"@types/js-yaml": "^4.0.2",
"@types/jsonwebtoken": "^8.5.4",
"@types/moxios": "^0.4.12",
"@types/qs": "^6.9.7",
"@types/react-copy-to-clipboard": "^5.0.1",
"@types/react-dom": "^17.0.9",
"@types/react-helmet": "^6.1.2",
"@types/react-redux": "^7.1.18",
"@types/react-router": "^5.1.16",
"@types/react-test-renderer": "^17.0.1",
"@types/react-router-dom": "^5.1.8",
"@types/react-router-hash-link": "^2.4.1",
"@types/react-transition-group": "^4.4.2",
"@types/redux-mock-store": "^1.0.3",
"@types/semver": "^7.3.8",
"@types/swagger-ui-react": "^3.35.2",
"enzyme": "^3.11.0",
"eslint-config-prettier": "^8.3.0",
Expand Down
51 changes: 35 additions & 16 deletions dashboard/src/components/AppList/AppList.test.tsx
@@ -1,14 +1,13 @@
import context from "jest-plugin-context";
import React from "react";
import * as ReactRedux from "react-redux";

import { deepClone } from "@cds/core/internal";
import actions from "actions";
import LoadingWrapper from "components/LoadingWrapper";
import SearchFilter from "components/SearchFilter/SearchFilter";
import * as qs from "qs";
import context from "jest-plugin-context";
import qs from "qs";
import React from "react";
import { act } from "react-dom/test-utils";
import * as ReactRouter from "react-router";
import * as ReactRedux from "react-redux";
import { MemoryRouter } from "react-router";
import { Kube } from "shared/Kube";
import { defaultStore, getStore, initialState, mountWrapper } from "shared/specs/mountWrapper";
import { FetchError, IAppOverview, IStoreState } from "../../shared/types";
Expand Down Expand Up @@ -57,15 +56,25 @@ context("when changing props", () => {
jest.spyOn(qs, "parse").mockReturnValue({
q: "foo",
});
const wrapper = mountWrapper(defaultStore, <AppList />);
const wrapper = mountWrapper(
defaultStore,
<MemoryRouter initialEntries={["/foo?q=foo"]}>
<AppList />
</MemoryRouter>,
Comment on lines +61 to +63
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main change here is replacing the jest.spyOn with the approach suggested in the official website: https://reactrouter.com/web/guides/testing

);
expect(wrapper.find(SearchFilter).prop("value")).toEqual("foo");
});

it("should list apps in all namespaces", () => {
jest.spyOn(qs, "parse").mockReturnValue({
allns: "yes",
});
const wrapper = mountWrapper(defaultStore, <AppList />);
const wrapper = mountWrapper(
defaultStore,
<MemoryRouter initialEntries={["/foo?allns=yes"]}>
<AppList />
</MemoryRouter>,
);
expect(wrapper.find("input[type='checkbox']")).toBeChecked();
});

Expand All @@ -92,10 +101,8 @@ context("when changing props", () => {

describe("when store changes", () => {
let spyOnUseState: jest.SpyInstance;
let spyOnUseLocation: jest.SpyInstance;
afterEach(() => {
spyOnUseState.mockRestore();
spyOnUseLocation.mockRestore();
});

it("should not set all-ns prop when getting changes in the namespace", async () => {
Expand All @@ -111,11 +118,13 @@ context("when changing props", () => {
}
return [init, useState];
});
spyOnUseLocation = jest.spyOn(ReactRouter, "useLocation").mockImplementation(() => {
return { pathname: "/foo", search: "allns=yes", state: undefined, hash: "" };
});

mountWrapper(defaultStore, <AppList />);
mountWrapper(
defaultStore,
<MemoryRouter initialEntries={["/foo?allns=yes"]}>
<AppList />
</MemoryRouter>,
);
expect(setAllNS).not.toHaveBeenCalledWith(false);
});
});
Expand Down Expand Up @@ -231,7 +240,12 @@ context("when apps available", () => {
jest.spyOn(qs, "parse").mockReturnValue({
q: "bar",
});
const wrapper = mountWrapper(getStore(state), <AppList />);
const wrapper = mountWrapper(
getStore(state),
<MemoryRouter initialEntries={["/foo?q=bar"]}>
<AppList />
</MemoryRouter>,
);
expect(wrapper.find(AppListItem).key()).toBe("foobar/bar");
});
});
Expand Down Expand Up @@ -273,7 +287,12 @@ context("when custom resources available", () => {
jest.spyOn(qs, "parse").mockReturnValue({
q: "nop",
});
const wrapper = mountWrapper(getStore(state), <AppList />);
const wrapper = mountWrapper(
getStore(state),
<MemoryRouter initialEntries={["/foo?q=nop"]}>
<AppList />
</MemoryRouter>,
);
const itemList = wrapper.find(CustomResourceListItem);
expect(itemList).not.toExist();
});
Expand Down
85 changes: 65 additions & 20 deletions dashboard/src/components/AppView/AppView.test.tsx
@@ -1,11 +1,10 @@
import * as yaml from "js-yaml";
import * as ReactRedux from "react-redux";
import * as ReactRouter from "react-router";

import actions from "actions";
import Alert from "components/js/Alert";
import LoadingWrapper from "components/LoadingWrapper/LoadingWrapper";
import PageHeader from "components/PageHeader";
import * as yaml from "js-yaml";
import * as ReactRedux from "react-redux";
import { MemoryRouter, Route } from "react-router";
import { defaultStore, getStore, mountWrapper } from "shared/specs/mountWrapper";
import { DeleteError, FetchError, IResource } from "shared/types";
import ApplicationStatusContainer from "../../containers/ApplicationStatusContainer";
Expand All @@ -22,8 +21,9 @@ const routeParams = {
namespace: "default",
releaseName: "mr-sunshine",
};
const routePathParam = `/foo/${routeParams.cluster}/${routeParams.namespace}/${routeParams.releaseName}`;
const routePath = "/foo/:cluster/:namespace/:releaseName";
let spyOnUseDispatch: jest.SpyInstance;
let spyOnUseParams: jest.SpyInstance;
const appActions = { ...actions.apps };
const kubeaActions = { ...actions.kube };

Expand All @@ -39,14 +39,12 @@ beforeEach(() => {
};
const mockDispatch = jest.fn();
spyOnUseDispatch = jest.spyOn(ReactRedux, "useDispatch").mockReturnValue(mockDispatch);
spyOnUseParams = jest.spyOn(ReactRouter, "useParams").mockReturnValue(routeParams);
});

afterEach(() => {
actions.apps = { ...appActions };
actions.kube = { ...kubeaActions };
spyOnUseDispatch.mockRestore();
spyOnUseParams.mockRestore();
});

describe("AppViewComponent", () => {
Expand Down Expand Up @@ -105,7 +103,11 @@ describe("AppViewComponent", () => {
it("renders a fetch error only", () => {
const wrapper = mountWrapper(
getStore({ apps: { error: new FetchError("boom!") } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);
expect(wrapper.find(Alert)).toExist();
expect(wrapper.find(PageHeader)).not.toExist();
Expand All @@ -126,7 +128,11 @@ describe("AppViewComponent", () => {

const wrapper = mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);

const tabs = wrapper.find(ResourceTabs);
Expand Down Expand Up @@ -189,7 +195,11 @@ describe("AppViewComponent", () => {

const wrapper = mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);
expect(watchResource).toHaveBeenCalledWith(depResource);
expect(watchResource).toHaveBeenCalledWith(svcResource);
Expand All @@ -208,7 +218,11 @@ describe("AppViewComponent", () => {

const wrapper = mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);

const tabs = wrapper.find(ResourceTabs);
Expand All @@ -231,7 +245,11 @@ describe("AppViewComponent", () => {

const wrapper = mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);

const tabs = wrapper.find(ResourceTabs);
Expand All @@ -255,7 +273,11 @@ describe("AppViewComponent", () => {
expect(() => {
mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);
}).not.toThrow();
});
Expand All @@ -271,7 +293,11 @@ describe("AppViewComponent", () => {
expect(() => {
const wrapper = mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);
const tabs = wrapper.find(ResourceTabs);
expect(tabs.prop("deployments")[0].name).toEqual("foo");
Expand All @@ -293,7 +319,11 @@ describe("AppViewComponent", () => {
it("renders an error if error prop is set", () => {
const wrapper = mountWrapper(
getStore({ ...validState, apps: { ...validState.apps, error: new Error("Boom!") } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);
const err = wrapper.find(Alert);
expect(err).toExist();
Expand All @@ -303,7 +333,11 @@ describe("AppViewComponent", () => {
it("renders a delete-error", () => {
const wrapper = mountWrapper(
getStore({ ...validState, apps: { ...validState.apps, error: new DeleteError("Boom!") } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);
const err = wrapper.find(Alert);
expect(err).toExist();
Expand All @@ -318,10 +352,13 @@ describe("AppViewComponent", () => {
items: [obj, resources.deployment],
};
const manifest = generateYamlManifest([resources.service, list]);

const wrapper = mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);

const tabs = wrapper.find(ResourceTabs);
Expand Down Expand Up @@ -360,7 +397,11 @@ describe("AppViewComponent", () => {

const wrapper = mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);

const tabs = wrapper.find(ResourceTabs);
Expand Down Expand Up @@ -394,7 +435,11 @@ describe("AppViewComponent", () => {
const manifest = generateYamlManifest(r);
const wrapper = mountWrapper(
getStore({ apps: { selected: { ...appRelease, manifest } } }),
<AppViewComponent />,
<MemoryRouter initialEntries={[routePathParam]}>
<Route path={routePath}>
<AppViewComponent />
</Route>
</MemoryRouter>,
);

const applicationStatus = wrapper.find(ApplicationStatusContainer);
Expand Down