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.

Sign up for GitHub

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

Jump to bottom

Code Quality Updates - Part 2 #240

Merged
merged 10 commits into from
Apr 15, 2024
Merged

Code Quality Updates - Part 2 #240

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
729df79
Fix server type test console errors.
jbouder Apr 11, 2024
b5b88c4
Add more tests.
jbouder Apr 11, 2024
0c6af9c
Merge branch 'main' into code-quality-updates-2
jbouder Apr 12, 2024
b9da144
Update app to set switch background based on theme.
jbouder Apr 12, 2024
3002b32
Fix app thumbnail background styling.
jbouder Apr 12, 2024
b6a203e
Fix test errors.
jbouder Apr 12, 2024
c85864c
Remove dev environment config from jest testing.
jbouder Apr 15, 2024
2645878
Add new build.
jbouder Apr 15, 2024
36f561d
Add and update unit tests for better coverage. Remove old prop from c…
jbouder Apr 15, 2024
43d2ab7
Add new build.
jbouder Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions jhub_apps/static/js/index.js
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions jhub_apps/templates/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,10 @@ a:focus {
outline-color: var(--primary-color) !important;
}

.form-section .thumbnail, .form-section .thumbnail-body.selected {
background-color: transparent !important;
}

/* Misc MUI */
.MuiButton-containedPrimary:not(:disabled) {
color: var(--light-text-color);
Expand Down Expand Up @@ -415,6 +419,10 @@ a:focus {
background-color: var(--white) !important;
}

.Mui-checked + .MuiSwitch-track {
background-color: var(--primary-color) !important;
}

.MuiOutlinedInput-root.Mui-focused > fieldset {
border-color: var(--primary-color) !important;
}
Expand Down
2 changes: 1 addition & 1 deletion ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"lint": "eslint src --ext ts,tsx --report-unused-disable-directives --max-warnings 0",
"preview": "vite preview",
"format": "npx prettier src --write",
"test": "NODE_ENV=development jest",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was something we unnecessarily added when implementing the dummy axios data, to support testing in vercel. It actually is not needed, and is resulting in a lot of unnecessary errors to be logged when running unit tests.

"test": "jest",
"coverage": "jest --coverage"
},
"dependencies": {
Expand Down
46 changes: 27 additions & 19 deletions ui/src/components/app-card/app-card.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is no longer used in the card, removing

serverStatus="ready"
/>
</QueryClientProvider>
Expand All @@ -49,15 +48,15 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
serverStatus="ready"
serverStatus="Pending"
/>
</QueryClientProvider>
</RecoilRoot>,
);
expect(baseElement).toBeInTheDocument();
const appLabel = baseElement.querySelector('.MuiTypography-h5');
expect(appLabel).toHaveTextContent('Test App');
expect(baseElement).toHaveTextContent('Pending');
});

test('renders default app card with no server status', async () => {
Expand All @@ -70,7 +69,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
serverStatus=""
/>
</QueryClientProvider>
Expand All @@ -91,8 +89,7 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
serverStatus="ready"
serverStatus="Ready"
isAppCard={false}
/>
</QueryClientProvider>
Expand All @@ -116,9 +113,8 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="ready"
serverStatus="Ready"
/>
</QueryClientProvider>
</RecoilRoot>,
Expand All @@ -142,7 +138,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Ready"
/>
Expand All @@ -153,13 +148,15 @@ describe('AppCard', () => {
const menu = baseElement.querySelectorAll(
'.MuiButtonBase-root',
)[0] as HTMLButtonElement;
expect(menu).toBeInTheDocument();
await act(async () => {
menu.click();
});

const btn = baseElement.querySelectorAll(
'.MuiList-root li.MuiButtonBase-root',
)[0] as HTMLAnchorElement;
expect(btn).toBeInTheDocument();
await act(async () => {
btn.click();
});
Expand All @@ -177,7 +174,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Ready"
/>
Expand All @@ -195,6 +191,9 @@ describe('AppCard', () => {
const btn = baseElement.querySelectorAll(
'.MuiList-root li.MuiButtonBase-root',
)[0] as HTMLAnchorElement;
expect(btn).toBeInTheDocument();
expect(btn).toHaveTextContent('Start');
expect(btn).not.toHaveAttribute('disabled', 'disabled');
await act(async () => {
btn.click();
});
Expand Down Expand Up @@ -223,7 +222,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Running"
/>
Expand Down Expand Up @@ -258,7 +256,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Running"
/>
Expand All @@ -276,6 +273,9 @@ describe('AppCard', () => {
const btn = baseElement.querySelectorAll(
'.MuiList-root li.MuiButtonBase-root',
)[1] as HTMLAnchorElement;
expect(btn).toBeInTheDocument();
expect(btn).toHaveTextContent('Stop');
expect(btn).not.toHaveAttribute('disabled', 'disabled');
await act(async () => {
btn.click();
});
Expand Down Expand Up @@ -314,7 +314,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Ready"
/>
Expand All @@ -331,13 +330,17 @@ describe('AppCard', () => {

const btn = baseElement.querySelectorAll(
'.MuiList-root li.MuiButtonBase-root',
)[0] as HTMLAnchorElement;
)[2] as HTMLAnchorElement;
expect(btn).toBeInTheDocument();
expect(btn).toHaveTextContent('Edit');
expect(btn).not.toHaveAttribute('disabled', 'disabled');
await act(async () => {
btn.click();
});
// TODO: Update this test when everything is running in single react app
expect(window.location.pathname).not.toBe('/edit-app/app-1');
});

test('simulates editing an app with an error', async () => {
mock.onGet(new RegExp('/frameworks')).reply(200, frameworks);
mock.onGet(new RegExp('/server/app-1')).reply(200, app);
Expand All @@ -354,7 +357,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Ready"
/>
Expand All @@ -371,7 +373,10 @@ describe('AppCard', () => {

const btn = baseElement.querySelectorAll(
'.MuiList-root li.MuiButtonBase-root',
)[0] as HTMLAnchorElement;
)[2] as HTMLAnchorElement;
expect(btn).toBeInTheDocument();
expect(btn).toHaveTextContent('Edit');
expect(btn).not.toHaveAttribute('disabled', 'disabled');
await act(async () => {
btn.click();
});
Expand All @@ -391,7 +396,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Ready"
/>
Expand All @@ -409,6 +413,9 @@ describe('AppCard', () => {
const btn = baseElement.querySelectorAll(
'.MuiList-root li.MuiButtonBase-root',
)[3] as HTMLAnchorElement;
expect(btn).toBeInTheDocument();
expect(btn).toHaveTextContent('Delete');
expect(btn).not.toHaveAttribute('disabled', 'disabled');
await act(async () => {
btn.click();
});
Expand All @@ -426,7 +433,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Ready"
/>
Expand All @@ -444,6 +450,9 @@ describe('AppCard', () => {
const btn = baseElement.querySelectorAll(
'.MuiList-root li.MuiButtonBase-root',
)[3] as HTMLAnchorElement;
expect(btn).toBeInTheDocument();
expect(btn).toHaveTextContent('Delete');
expect(btn).not.toHaveAttribute('disabled', 'disabled');
await act(async () => {
btn.click();
});
Expand Down Expand Up @@ -479,7 +488,6 @@ describe('AppCard', () => {
username="Developer"
framework="Some Framework"
url="/some-url"
ready={true}
thumbnail="/some-thumbnail.png"
serverStatus="Ready"
/>
Expand Down
1 change: 0 additions & 1 deletion ui/src/components/app-card/app-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ interface AppCardProps {
thumbnail?: string;
url: string;
username?: string;
ready?: boolean;
isPublic?: boolean;
isShared?: boolean;
serverStatus: string;
Expand Down
Loading
Loading