Skip to content

Commit 47bdca7

Browse files
benoitfbenoitf
authored
fix: add back the error if an extension fails (podman-desktop#6940)
* fix: add back the error if an extension fails fixes podman-desktop#6917 Signed-off-by: Florent Benoit <[email protected]>
1 parent 141210f commit 47bdca7

10 files changed

+207
-6
lines changed

‎packages/renderer/src/lib/extensions/ExtensionDetails.spec.ts

+46-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818

1919
import '@testing-library/jest-dom/vitest';
2020

21-
import { render, screen } from '@testing-library/svelte';
21+
import { fireEvent, render, screen } from '@testing-library/svelte';
2222
import { beforeEach, expect, test, vi } from 'vitest';
2323

2424
import { type CombinedExtensionInfoUI } from '/@/stores/all-installed-extensions';
@@ -104,6 +104,20 @@ const combined: CombinedExtensionInfoUI[] = [
104104
},
105105
] as unknown[] as CombinedExtensionInfoUI[];
106106

107+
const combinedWithError: CombinedExtensionInfoUI[] = [
108+
{
109+
id: 'idAInstalled',
110+
displayName: 'A failed installed Extension',
111+
name: 'A extension',
112+
removable: true,
113+
state: 'failed',
114+
error: {
115+
message: 'custom error',
116+
stack: 'custom stack',
117+
},
118+
},
119+
] as unknown[] as CombinedExtensionInfoUI[];
120+
107121
test('Expect to have details page', async () => {
108122
const extensionId = 'idAInstalled';
109123

@@ -120,6 +134,37 @@ test('Expect to have details page', async () => {
120134

121135
const extensionBadge = screen.getByRole('region', { name: 'Extension Badge' });
122136
expect(extensionBadge).toBeInTheDocument();
137+
138+
// no tabs as not failing state
139+
const readmeTab = screen.queryByRole('button', { name: 'Readme' });
140+
expect(readmeTab).not.toBeInTheDocument();
141+
const errorTab = screen.queryByRole('button', { name: 'Error' });
142+
expect(errorTab).not.toBeInTheDocument();
143+
});
144+
145+
test('Expect to have details page with error tab with failed state', async () => {
146+
const extensionId = 'idAInstalled';
147+
148+
catalogExtensionInfos.set([aFakeExtension]);
149+
extensionInfos.set(combinedWithError);
150+
151+
await waitRender({ extensionId });
152+
153+
// check that we have two tabs
154+
const readmeTab = screen.getByRole('button', { name: 'Readme' });
155+
expect(readmeTab).toBeInTheDocument();
156+
157+
// check that we have two tabs
158+
const errorTab = screen.getByRole('button', { name: 'Error' });
159+
expect(errorTab).toBeInTheDocument();
160+
161+
// click on the error tab
162+
await fireEvent.click(errorTab);
163+
164+
// now check that the error is on the page
165+
// should contain the error
166+
const error = screen.getByText('Error: custom error');
167+
expect(error).toBeInTheDocument();
123168
});
124169

125170
test('Expect empty screen', async () => {

‎packages/renderer/src/lib/extensions/ExtensionDetails.svelte

+27-2
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,21 @@ import { combinedInstalledExtensions } from '/@/stores/all-installed-extensions'
77
import { catalogExtensionInfos } from '/@/stores/catalog-extensions';
88
99
import FeaturedExtensionDownload from '../featured/FeaturedExtensionDownload.svelte';
10+
import Button from '../ui/Button.svelte';
1011
import DetailsPage from '../ui/DetailsPage.svelte';
1112
import EmptyScreen from '../ui/EmptyScreen.svelte';
1213
import ExtensionStatus from '../ui/ExtensionStatus.svelte';
1314
import type { ExtensionDetailsUI } from './extension-details-ui';
1415
import ExtensionBadge from './ExtensionBadge.svelte';
16+
import ExtensionDetailsError from './ExtensionDetailsError.svelte';
1517
import ExtensionDetailsReadme from './ExtensionDetailsReadme.svelte';
1618
import ExtensionDetailsSummaryCard from './ExtensionDetailsSummaryCard.svelte';
1719
import { ExtensionsUtils } from './extensions-utils';
1820
import InstalledExtensionActions from './InstalledExtensionActions.svelte';
1921
2022
export let extensionId: string;
2123
24+
let screen: 'README' | 'ERROR' = 'README';
2225
let detailsPage: DetailsPage;
2326
const extensionsUtils = new ExtensionsUtils();
2427
@@ -67,10 +70,32 @@ $: extension = derived(
6770
<div slot="detail" class="flex">
6871
<ExtensionBadge class="mt-2" extension="{$extension}" />
6972
</div>
73+
<!-- Display tabs only if extension is in error state -->
74+
<svelte:fragment slot="tabs">
75+
{#if $extension.state === 'failed'}
76+
<Button
77+
type="tab"
78+
on:click="{() => {
79+
screen = 'README';
80+
}}"
81+
selected="{screen === 'README'}">Readme</Button>
82+
<Button
83+
type="tab"
84+
on:click="{() => {
85+
screen = 'ERROR';
86+
}}"
87+
selected="{screen === 'ERROR'}">Error</Button>
88+
{/if}
89+
</svelte:fragment>
90+
7091
<svelte:fragment slot="content">
7192
<div class="flex w-full h-full overflow-y-auto p-4 flex-col lg:flex-row">
72-
<ExtensionDetailsSummaryCard extensionDetails="{$extension}" />
73-
<ExtensionDetailsReadme readme="{$extension.readme}" />
93+
{#if screen === 'README'}
94+
<ExtensionDetailsSummaryCard extensionDetails="{$extension}" />
95+
<ExtensionDetailsReadme readme="{$extension.readme}" />
96+
{:else if screen === 'ERROR'}
97+
<ExtensionDetailsError extension="{$extension}" />
98+
{/if}
7499
</div>
75100
</svelte:fragment>
76101
</DetailsPage>

‎packages/renderer/src/lib/extensions/ExtensionDetailsError.spec.ts

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**********************************************************************
2+
* Copyright (C) 2024 Red Hat, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-License-Identifier: Apache-2.0
17+
***********************************************************************/
18+
19+
import '@testing-library/jest-dom/vitest';
20+
21+
import { render, screen } from '@testing-library/svelte';
22+
import { beforeEach, expect, test, vi } from 'vitest';
23+
24+
import type { ExtensionDetailsUI } from './extension-details-ui';
25+
import ExtensionDetailsError from './ExtensionDetailsError.svelte';
26+
27+
beforeEach(() => {
28+
vi.resetAllMocks();
29+
});
30+
31+
test('Expect to have error message being displayed', async () => {
32+
const extension: ExtensionDetailsUI = {
33+
displayName: 'my display name',
34+
description: 'my description',
35+
type: 'pd',
36+
removable: false,
37+
state: 'started',
38+
name: 'foo',
39+
icon: 'fooIcon',
40+
readme: { content: '' },
41+
releaseDate: '2024-01-01',
42+
categories: ['cat1', 'cat2'],
43+
publisherDisplayName: 'my publisher',
44+
version: 'v1.2.3',
45+
id: 'myId',
46+
fetchable: true,
47+
fetchLink: 'myLink',
48+
fetchVersion: 'v3.4.5',
49+
error: {
50+
message: 'An error occurred',
51+
stack: 'line1\nline2',
52+
},
53+
};
54+
55+
render(ExtensionDetailsError, { extension });
56+
57+
// should contain the error
58+
const error = screen.getByText('Error: An error occurred');
59+
expect(error).toBeInTheDocument();
60+
61+
// should contain the stack
62+
const stack = screen.getByRole('group', { name: 'Stack Trace' });
63+
expect(stack).toBeInTheDocument();
64+
65+
// should contain the stack lines
66+
expect(stack).toContainHTML('line1\nline2');
67+
});

‎packages/renderer/src/lib/extensions/ExtensionDetailsError.svelte

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<script lang="ts">
2+
import type { ExtensionDetailsUI } from './extension-details-ui';
3+
4+
export let extension: ExtensionDetailsUI;
5+
</script>
6+
7+
{#if extension.error}
8+
<div class="flex flex-col">
9+
<div class="py-2">Error: {extension.error.message}</div>
10+
{#if extension.error.stack}
11+
<div class="text-xs">
12+
<div>Stack trace:</div>
13+
<pre role="group" aria-label="Stack Trace">{extension.error.stack}</pre>
14+
</div>
15+
{/if}
16+
</div>
17+
{/if}

‎packages/renderer/src/lib/extensions/InstalledExtensionCardLeftLifecycleStart.spec.ts

+27
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,30 @@ test('Expect unable to start if already started', async () => {
104104
const button = screen.queryByRole('button', { name: 'Start' });
105105
expect(button).not.toBeInTheDocument();
106106
});
107+
108+
test('Expect to start Extension if failed', async () => {
109+
const extension: CombinedExtensionInfoUI = {
110+
type: 'pd',
111+
id: 'idExtension',
112+
name: 'fooName',
113+
description: 'my description',
114+
displayName: '',
115+
publisher: '',
116+
removable: true,
117+
version: 'v1.2.3',
118+
state: 'failed',
119+
path: '',
120+
readme: '',
121+
};
122+
render(InstalledExtensionCardLeftLifecycleStart, { extension });
123+
124+
// get button with label 'Start'
125+
const button = screen.getByRole('button', { name: 'Start' });
126+
expect(button).toBeInTheDocument();
127+
128+
// click the button
129+
await fireEvent.click(button);
130+
131+
// expect the start function to be called
132+
expect(vi.mocked(window.startExtension)).toHaveBeenCalledWith('idExtension');
133+
});

‎packages/renderer/src/lib/extensions/InstalledExtensionCardLeftLifecycleStart.svelte

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ async function startExtension(): Promise<void> {
1616
}
1717
</script>
1818

19-
{#if extension.state === 'stopped'}
19+
{#if extension.state === 'stopped' || extension.state === 'failed'}
2020
<LoadingIconButton
2121
clickAction="{() => startExtension()}"
2222
action="start"

‎packages/renderer/src/lib/extensions/extension-details-ui.ts

+3
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818

1919
import type { CombinedExtensionInfoUI } from '/@/stores/all-installed-extensions';
2020

21+
import type { ExtensionError } from '../../../../main/src/plugin/api/extension-info';
22+
2123
export interface ExtensionDetailsUI {
2224
displayName: string;
2325
description: string;
@@ -37,4 +39,5 @@ export interface ExtensionDetailsUI {
3739
fetchable: boolean;
3840
fetchLink: string;
3941
fetchVersion: string;
42+
error?: ExtensionError;
4043
}

‎packages/renderer/src/lib/extensions/extensions-utils.spec.ts

+15
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,10 @@ const installedExtensions: CombinedExtensionInfoUI[] = [
179179
id: 'idAInstalled',
180180
displayName: 'A installed Extension',
181181
removable: true,
182+
error: {
183+
message: 'An error occurred',
184+
stack: 'line1\nline2',
185+
},
182186
},
183187
{
184188
id: 'idYInstalled',
@@ -292,4 +296,15 @@ describe('extractExtensionDetail', () => {
292296
expect(extensionDetail?.publisherDisplayName).toBe('Foo Publisher');
293297
expect(extensionDetail?.version).toBe('v1.0.0Z');
294298
});
299+
300+
test('Check error is kept', async () => {
301+
const extensionDetail = extensionsUtils.extractExtensionDetail(
302+
catalogExtensions,
303+
installedExtensions,
304+
'idAInstalled',
305+
);
306+
expect(extensionDetail).toBeDefined();
307+
expect(extensionDetail?.error?.message).toBe('An error occurred');
308+
expect(extensionDetail?.error?.stack).toBe('line1\nline2');
309+
});
295310
});

‎packages/renderer/src/lib/extensions/extensions-utils.ts

+2
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,7 @@ export class ExtensionsUtils {
113113
const version = matchingInstalledExtensionVersion ?? latestVersionNumber ?? 'N/A';
114114

115115
const installedExtension = matchingInstalledExtension;
116+
const error = matchingInstalledExtension?.error;
116117

117118
const fetchLink = latestVersionOciLink ?? '';
118119
const fetchVersion = latestVersion?.version ?? '';
@@ -138,6 +139,7 @@ export class ExtensionsUtils {
138139
fetchable,
139140
fetchLink,
140141
fetchVersion,
142+
error,
141143
};
142144
return matchingExtension;
143145
}

‎packages/renderer/src/lib/ui/LoadingIconButton.svelte

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export let clickAction: () => Promise<void> | void;
1717
$: disable =
1818
state?.inProgress ||
1919
state?.status === 'unsupported' ||
20-
(action === 'start' && state?.status !== 'stopped') ||
20+
(action === 'start' && state?.status !== 'stopped' && state?.status !== 'failed') ||
2121
(action === 'restart' && state?.status !== 'started') ||
2222
(action === 'stop' && state?.status !== 'started') ||
2323
(action === 'delete' && state?.status !== 'stopped' && state?.status !== 'unknown') ||
@@ -27,7 +27,7 @@ $: loading = state?.inProgress && action === state?.action;
2727
2828
function getStyleByState(state: ILoadingStatus | undefined, action: string): string {
2929
if (
30-
(action === 'start' && (state?.inProgress || state?.status !== 'stopped')) ||
30+
(action === 'start' && (state?.inProgress || (state?.status !== 'stopped' && state?.status !== 'failed'))) ||
3131
((action === 'stop' || action === 'restart') && (state?.inProgress || state?.status !== 'started')) ||
3232
(action === 'delete' && (state?.inProgress || (state?.status !== 'stopped' && state?.status !== 'unknown'))) ||
3333
(action === 'update' && (state?.inProgress || state?.status === 'unknown')) ||

0 commit comments

Comments
 (0)