Skip to content

Commit 263ee74

Browse files
axel7083danivilla9
axel7083
authored and
danivilla9
committed
refactor(github-feedback): removing test ids and factoring code (podman-desktop#10117)
* refactor(github-feedback): removing test ids and factoring code Signed-off-by: axel7083 <[email protected]> * fix: removing useless links Signed-off-by: axel7083 <[email protected]> --------- Signed-off-by: axel7083 <[email protected]>
1 parent 04d32f9 commit 263ee74

File tree

2 files changed

+126
-76
lines changed

2 files changed

+126
-76
lines changed

‎packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.spec.ts‎

Lines changed: 114 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@
1818

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

21-
import { render, screen } from '@testing-library/svelte';
21+
import { render, type RenderResult } from '@testing-library/svelte';
2222
import userEvent from '@testing-library/user-event';
23-
import { tick } from 'svelte';
23+
import { type Component, type ComponentProps } from 'svelte';
2424
import { beforeAll, beforeEach, expect, test, vi } from 'vitest';
2525

26-
import type { GitHubIssue } from '/@api/feedback';
26+
import type { FeedbackCategory } from '/@api/feedback';
2727

2828
import GitHubIssueFeedback from './GitHubIssueFeedback.svelte';
2929

@@ -49,98 +49,138 @@ beforeEach(() => {
4949
vi.resetAllMocks();
5050
});
5151

52+
/**
53+
* Utility method to query get GitHub issue inputs
54+
* @param props
55+
*/
56+
function renderGitHubIssueFeedback(props: ComponentProps<typeof GitHubIssueFeedback>): {
57+
title: HTMLInputElement;
58+
description: HTMLTextAreaElement;
59+
preview: HTMLButtonElement;
60+
} & RenderResult<Component<ComponentProps<typeof GitHubIssueFeedback>>> {
61+
const { getByRole, queryByTitle, ...restResult } = render(GitHubIssueFeedback, props);
62+
63+
// text inputs
64+
const title = getByRole('textbox', { name: 'Issue Title' });
65+
expect(title).toBeInstanceOf(HTMLInputElement);
66+
67+
const description = getByRole('textbox', { name: 'Issue Description' });
68+
expect(description).toBeInstanceOf(HTMLTextAreaElement);
69+
70+
// button
71+
const preview = getByRole('button', { name: 'Preview on GitHub' });
72+
expect(preview).toBeInstanceOf(HTMLButtonElement);
73+
74+
return {
75+
title: title as HTMLInputElement,
76+
description: description as HTMLTextAreaElement,
77+
preview: preview as HTMLButtonElement,
78+
getByRole,
79+
queryByTitle,
80+
...restResult,
81+
};
82+
}
83+
5284
test('Expect feedback form to to be GitHub issue feedback form', async () => {
53-
const renderObject = render(GitHubIssueFeedback, { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn() });
85+
const { title, description } = renderGitHubIssueFeedback({
86+
category: 'bug',
87+
onCloseForm: vi.fn(),
88+
contentChange: vi.fn(),
89+
});
5490

55-
expect(screen.getByText('Title')).toBeInTheDocument();
56-
expect(screen.getByText('Description')).toBeInTheDocument();
57-
renderObject.unmount();
91+
expect(title).toBeInTheDocument();
92+
expect(description).toBeInTheDocument();
5893
});
5994

6095
test('Expect Preview on GitHub button to be disabled if there is no title or description', async () => {
61-
const renderObject = render(GitHubIssueFeedback, { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn() });
62-
63-
expect(screen.getByRole('button', { name: 'Preview on GitHub' })).toBeDisabled();
96+
const { title, description, preview } = renderGitHubIssueFeedback({
97+
category: 'bug',
98+
onCloseForm: vi.fn(),
99+
contentChange: vi.fn(),
100+
});
64101

65-
const issueTitle = screen.getByTestId('issueTitle');
66-
expect(issueTitle).toBeInTheDocument();
67-
await userEvent.type(issueTitle, 'Bug title');
102+
// default: disabled
103+
expect(preview).toBeDisabled();
68104

69-
const issueDescription = screen.getByTestId('issueDescription');
70-
expect(issueDescription).toBeInTheDocument();
71-
await userEvent.type(issueDescription, 'Bug description');
105+
await userEvent.type(title, 'Bug title');
106+
await userEvent.type(description, 'Bug description');
72107

73-
await tick();
74-
expect(screen.getByRole('button', { name: 'Preview on GitHub' })).toBeEnabled();
75-
renderObject.unmount();
108+
await vi.waitFor(() => {
109+
expect(preview).toBeEnabled();
110+
});
76111
});
77112

78-
test('Expect to have different placeholders for bug vs feaure', async () => {
79-
let renderObject = render(GitHubIssueFeedback, { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn() });
80-
81-
let issueTitle = screen.getByTestId('issueTitle');
82-
expect(issueTitle).toBeInTheDocument();
83-
expect(issueTitle).toHaveProperty('placeholder', 'Bug Report Title');
84-
85-
let issueDescription = screen.getByTestId('issueDescription');
86-
expect(issueDescription).toBeInTheDocument();
87-
expect(issueDescription).toHaveProperty('placeholder', 'Bug description');
88-
89-
renderObject.unmount();
90-
91-
renderObject = render(GitHubIssueFeedback, { category: 'feature', onCloseForm: vi.fn(), contentChange: vi.fn() });
92-
93-
issueTitle = screen.getByTestId('issueTitle');
94-
expect(issueTitle).toBeInTheDocument();
95-
expect(issueTitle).toHaveProperty('placeholder', 'Feature name');
113+
test.each([
114+
{
115+
category: 'bug',
116+
placeholders: {
117+
title: 'Bug Report Title',
118+
description: 'Bug description',
119+
},
120+
},
121+
{
122+
category: 'feature',
123+
placeholders: {
124+
title: 'Feature name',
125+
description: 'Feature description',
126+
},
127+
},
128+
])('$category should have specific placeholders', async ({ category, placeholders }) => {
129+
const { title, description } = renderGitHubIssueFeedback({
130+
category: category as FeedbackCategory,
131+
onCloseForm: vi.fn(),
132+
contentChange: vi.fn(),
133+
});
96134

97-
issueDescription = screen.getByTestId('issueDescription');
98-
expect(issueDescription).toBeInTheDocument();
99-
expect(issueDescription).toHaveProperty('placeholder', 'Feature description');
100-
renderObject.unmount();
135+
expect(title).toHaveProperty('placeholder', placeholders.title);
136+
expect(description).toHaveProperty('placeholder', placeholders.description);
101137
});
102138

103-
test('Expect to have different existing GitHub issues links for bug and feature categories', async () => {
104-
let renderObject = render(GitHubIssueFeedback, { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn() });
105-
let existingIssues = screen.getByLabelText('GitHub issues');
106-
expect(existingIssues).toBeInTheDocument();
107-
108-
await userEvent.click(existingIssues);
109-
expect(openExternalMock).toHaveBeenCalledWith(
110-
'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Fbug%20%F0%9F%90%9E%22',
111-
);
112-
renderObject.unmount();
139+
test.each([
140+
{
141+
category: 'bug',
142+
link: 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Fbug%20%F0%9F%90%9E%22',
143+
},
144+
{
145+
category: 'feature',
146+
link: 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Ffeature%20%F0%9F%92%A1%22',
147+
},
148+
])('$category should have specific issues link', async ({ category, link }) => {
149+
const { getByLabelText } = renderGitHubIssueFeedback({
150+
category: category as FeedbackCategory,
151+
onCloseForm: vi.fn(),
152+
contentChange: vi.fn(),
153+
});
113154

114-
renderObject = render(GitHubIssueFeedback, { category: 'feature', onCloseForm: vi.fn(), contentChange: vi.fn() });
115-
existingIssues = screen.getByLabelText('GitHub issues');
155+
const existingIssues = getByLabelText('GitHub issues');
116156
expect(existingIssues).toBeInTheDocument();
117157

118158
await userEvent.click(existingIssues);
119-
expect(openExternalMock).toHaveBeenCalledWith(
120-
'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Ffeature%20%F0%9F%92%A1%22',
121-
);
122-
renderObject.unmount();
159+
expect(openExternalMock).toHaveBeenCalledWith(link);
123160
});
124161

125-
test('Expect the right category to be included in previewOnGitHub call', async () => {
126-
const issueProperties: GitHubIssue = {
127-
category: 'bug',
128-
title: 'Bug title',
129-
description: 'Bug description',
130-
};
131-
const renderObject = render(GitHubIssueFeedback, { category: 'bug', onCloseForm: vi.fn(), contentChange: vi.fn() });
132-
const previewButton = screen.getByRole('button', { name: 'Preview on GitHub' });
162+
test.each(['bug', 'feature'])('Expect %s to be included in previewOnGitHub call', async category => {
163+
const { preview, title, description } = renderGitHubIssueFeedback({
164+
category: category as FeedbackCategory,
165+
onCloseForm: vi.fn(),
166+
contentChange: vi.fn(),
167+
});
133168

134-
const issueTitle = screen.getByTestId('issueTitle');
135-
expect(issueTitle).toBeInTheDocument();
136-
await userEvent.type(issueTitle, 'Bug title');
169+
// type dummy data
170+
await userEvent.type(title, 'Bug title');
171+
await userEvent.type(description, 'Bug description');
137172

138-
const issueDescription = screen.getByTestId('issueDescription');
139-
expect(issueDescription).toBeInTheDocument();
140-
await userEvent.type(issueDescription, 'Bug description');
173+
// wait enable
174+
await vi.waitFor(() => {
175+
expect(preview).toBeEnabled();
176+
});
141177

142-
await userEvent.click(previewButton);
178+
// preview
179+
await userEvent.click(preview);
143180

144-
expect(previewOnGitHubMock).toHaveBeenCalledWith(issueProperties);
145-
renderObject.unmount();
181+
expect(previewOnGitHubMock).toHaveBeenCalledWith(
182+
expect.objectContaining({
183+
category: category,
184+
}),
185+
);
146186
});

‎packages/renderer/src/lib/feedback/feedbackForms/GitHubIssueFeedback.svelte‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,16 +62,26 @@ async function previewOnGitHub(): Promise<void> {
6262
<FeedbackForm>
6363
<svelte:fragment slot="content">
6464
<div class="text-sm text-[var(--pd-state-warning)]">You can search existing {category} issues on <Link aria-label="GitHub issues" onclick={openGitHubIssues}>github.com/podman-desktop/podman-desktop/issues</Link></div>
65+
<!-- issue title -->
6566
<label for="issueTitle" class="block mt-4 mb-2 text-sm font-medium text-[var(--pd-modal-text)]">Title</label>
66-
<input type="text" name="issueTitle" id="issueTitle" bind:value={issueTitle} data-testid="issueTitle" placeholder={titlePlaceholder} class="w-full p-2 outline-none text-sm bg-[var(--pd-input-field-focused-bg)] rounded-sm text-[var(--pd-input-field-focused-text)] placeholder-[var(--pd-input-field-placeholder-text)]"/>
67+
<input
68+
type="text"
69+
name="issueTitle"
70+
id="issueTitle"
71+
aria-label="Issue Title"
72+
bind:value={issueTitle}
73+
placeholder={titlePlaceholder}
74+
class="w-full p-2 outline-none text-sm bg-[var(--pd-input-field-focused-bg)] rounded-sm text-[var(--pd-input-field-focused-text)] placeholder-[var(--pd-input-field-placeholder-text)]"/>
75+
76+
<!-- issue body -->
6777
<label for="issueDescription" class="block mt-4 mb-2 text-sm font-medium text-[var(--pd-modal-text)]"
6878
>Description</label>
6979
<textarea
7080
rows="3"
7181
maxlength="1000"
7282
name="issueDescription"
83+
aria-label="Issue Description"
7384
id="issueDescription"
74-
data-testid="issueDescription"
7585
bind:value={issueDescription}
7686
class="w-full p-2 outline-none text-sm bg-[var(--pd-input-field-focused-bg)] rounded-sm text-[var(--pd-input-field-focused-text)] placeholder-[var(--pd-input-field-placeholder-text)]"
7787
placeholder={descriptionPlaceholder}></textarea>

0 commit comments

Comments
 (0)