codeant-ai-for-open-source[bot] commented on code in PR #41772:
URL: https://github.com/apache/superset/pull/41772#discussion_r3524362630
##########
superset-frontend/plugins/plugin-chart-table/test/TableChart.test.tsx:
##########
@@ -2383,6 +2383,75 @@ describe('plugin-chart-table', () => {
expect(filters[0].val).toEqual(['Michael']);
});
});
+
+ describe('Search Select Dropdown conditionally rendering', () => {
+ it('does not render "Search by" if include_search is false (server
pagination enabled)', () => {
+ const props = transformProps({
+ ...testData.raw,
+ rawFormData: {
+ ...testData.raw.rawFormData,
+ server_pagination: true,
+ include_search: false,
+ },
+ });
+ render(
+ ProviderWrapper({
+ children: <TableChart {...props} sticky={false} />,
+ }),
+ );
+ expect(screen.queryByText('Search by:')).not.toBeInTheDocument();
+ });
+
+ it('does not render "Search by" if there are no search options (server
pagination enabled)', () => {
+ const props = transformProps({
+ ...testData.raw,
+ rawFormData: {
+ ...testData.raw.rawFormData,
+ server_pagination: true,
+ include_search: true,
+ },
+ queriesData: [
+ {
+ ...testData.raw.queriesData[0],
+ colnames: ['num'],
+ coltypes: [GenericDataType.Numeric],
+ data: [{ num: 1 }, { num: 2 }],
+ },
+ ],
+ });
+ render(
+ ProviderWrapper({
+ children: <TableChart {...props} sticky={false} />,
+ }),
+ );
+ expect(screen.queryByText('Search by:')).not.toBeInTheDocument();
+ });
+
+ it('renders "Search by" if include_search is true and there are search
options (server pagination enabled)', () => {
+ const props = transformProps({
+ ...testData.raw,
+ rawFormData: {
+ ...testData.raw.rawFormData,
+ server_pagination: true,
+ include_search: true,
+ },
+ queriesData: [
+ {
+ ...testData.raw.queriesData[0],
+ colnames: ['name'],
+ coltypes: [GenericDataType.String],
+ data: [{ name: 'Michael' }, { name: 'John' }],
+ },
+ ],
+ });
+ render(
+ ProviderWrapper({
+ children: <TableChart {...props} sticky={false} />,
+ }),
+ );
+ expect(screen.queryByText('Search by:')).toBeInTheDocument();
+ });
+ });
Review Comment:
Yes — this is a valid style nit.
The new `describe('Search Select Dropdown conditionally rendering', ...)`
wrapper isn’t necessary here, and it goes against the “avoid nesting” testing
guideline. These three cases can be flattened into standalone `test()` blocks
at the same level as the surrounding tests, while keeping the same assertions
and setup.
For example:
```ts
test('does not render "Search by" if include_search is false (server
pagination enabled)', () => {
// ...
});
test('does not render "Search by" if there are no search options (server
pagination enabled)', () => {
// ...
});
test('renders "Search by" if include_search is true and there are search
options (server pagination enabled)', () => {
// ...
});
```
That keeps the tests easy to scan and avoids an extra grouping layer for
only three cases.
The behavior being tested is still important, so the coverage should stay —
just flattened inline. If you want, I can also help adjust the rest of the PR
comments in the same style.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
