From 248f521f07cf93f20342bedbe734bf6f93c5a30b Mon Sep 17 00:00:00 2001 From: Shailesh Parmar Date: Sun, 7 Sep 2025 10:23:39 +0530 Subject: [PATCH] fix: only newly added test case was only getting selected in pipeline while creating new test case (#23279) * fix: only newly added test case was getting selected in pipeline while creating new test case * feat: add functionality to create and validate multiple test cases from table details page * fix: refactor column selection logic to improve test case flow and reduce flakiness --- .../DataQuality/AddTestCaseNewFlow.spec.ts | 145 +++++++++++++++--- .../components/TestCaseFormV1.test.tsx | 51 ++++++ .../components/TestCaseFormV1.tsx | 7 +- .../TableProfilerProvider.test.tsx | 88 ++++++++++- .../TableProfiler/TableProfilerProvider.tsx | 19 ++- 5 files changed, 278 insertions(+), 32 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/AddTestCaseNewFlow.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/AddTestCaseNewFlow.spec.ts index 88eae05c384..964658b90e6 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/AddTestCaseNewFlow.spec.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/DataQuality/AddTestCaseNewFlow.spec.ts @@ -32,6 +32,16 @@ test.describe('Add TestCase New Flow', () => { .click(); }; + const selectColumn = async (page: Page, columnName: string) => { + await page.click('#testCaseFormV1_selectedColumn'); + // appearing dropdown takes bit time and its not based on API call so adding manual wait to prevent flakiness. + await page.waitForTimeout(2000); + await page.waitForSelector(`.ant-select-dropdown [title="${columnName}"]`, { + state: 'visible', + }); + await page.locator(`.ant-select-dropdown [title="${columnName}"]`).click(); + }; + // Helper function to create test case const createTestCase = async (data: { page: Page; @@ -81,6 +91,11 @@ test.describe('Add TestCase New Flow', () => { const response = await tableTestCaseResponse; const ingestionPipelineResponse = await ingestionPipeline; + const requestBody = JSON.parse( + ingestionPipelineResponse.request().postData() || '{}' + ); + + expect(requestBody?.sourceConfig?.config).not.toHaveProperty('testCases'); expect(response.status()).toBe(201); expect(ingestionPipelineResponse.status()).toBe(201); } else { @@ -122,16 +137,22 @@ test.describe('Add TestCase New Flow', () => { await redirectToHomePage(page); }); + const tableTestCaseDetails = { + testType: 'table row count to equal', + testTypeId: 'tableRowCountToEqual', + paramsValue: '10', + }; + + const columnTestCaseDetails = { + testType: 'Column Values To Be Unique', + testTypeId: 'columnValuesToBeUnique', + }; + test('Add Table Test Case', async ({ page }) => { const table = new TableClass(); const { apiContext } = await getApiContext(page); await table.create(apiContext); - const testCaseDetails = { - testType: 'table row count to equal', - testTypeId: 'tableRowCountToEqual', - paramsValue: '10', - }; await visitDataQualityPage(page); await test.step('Create table-level test case', async () => { @@ -140,11 +161,11 @@ test.describe('Add TestCase New Flow', () => { await selectTable(page, table); await createTestCase({ page, - ...testCaseDetails, + ...tableTestCaseDetails, }); await expect(page.getByTestId('entity-header-name')).toHaveText( - `${testCaseDetails.testTypeId}_test_case` + `${tableTestCaseDetails.testTypeId}_test_case` ); }); @@ -179,10 +200,6 @@ test.describe('Add TestCase New Flow', () => { await visitDataQualityPage(page); await test.step('Create column-level test case', async () => { - const testCaseDetails = { - testType: 'Column Values To Be Unique', - testTypeId: 'columnValuesToBeUnique', - }; await visitDataQualityPage(page); // Create column-level test case await openTestCaseForm(page); @@ -192,25 +209,15 @@ test.describe('Add TestCase New Flow', () => { .click(); await selectTable(page, table); - await page.click('#testCaseFormV1_selectedColumn'); - // appearing dropdown takes bit time and its not based on API call so adding manual wait to prevent flakiness. - await page.waitForTimeout(2000); - await page.waitForSelector( - `.ant-select-dropdown [title="${table.entity.columns[0].name}"]` - ); - await page - .locator( - `.ant-select-dropdown [title="${table.entity.columns[0].name}"]` - ) - .click(); + await selectColumn(page, table.entity.columns[0].name); await createTestCase({ page, - ...testCaseDetails, + ...columnTestCaseDetails, }); await expect(page.getByTestId('entity-header-name')).toHaveText( - `${testCaseDetails.testTypeId}_test_case` + `${columnTestCaseDetails.testTypeId}_test_case` ); }); @@ -237,6 +244,96 @@ test.describe('Add TestCase New Flow', () => { }); }); + test('Add multiple test case from table details page and validate pipeline', async ({ + page, + }) => { + test.slow(); + + const table = new TableClass(); + const { apiContext } = await getApiContext(page); + await table.create(apiContext); + + await visitDataQualityTab(page, table); + + await page + .getByRole('menuitem', { + name: 'Data Quality', + }) + .click(); + + await page.waitForLoadState('networkidle'); + await page.waitForSelector('[data-testid="loader"]', { state: 'detached' }); + + await page.click('[data-testid="profiler-add-table-test-btn"]'); + await page.click('[data-testid="table"]'); + await page.waitForLoadState('networkidle'); + + await createTestCase({ + page, + ...tableTestCaseDetails, + }); + + await page.click('[data-testid="profiler-add-table-test-btn"]'); + await page.click('[data-testid="column"]'); + await page.waitForLoadState('networkidle'); + + await selectColumn(page, table.entity.columns[0].name); + + await createTestCase({ + page, + ...columnTestCaseDetails, + expectSchedulerCard: false, + }); + + await page.waitForSelector('[data-testid="test-case-form-v1"]', { + state: 'detached', + }); + + await expect( + page.getByTestId('test-cases').getByTestId('count') + ).toHaveText('2'); + + await expect(page.getByTestId('pipeline').getByTestId('count')).toHaveText( + '1' + ); + + const pipelineApi = page.waitForResponse( + '/api/v1/services/ingestionPipelines?*pipelineType=TestSuite*' + ); + await page.getByTestId('pipeline').click(); + await pipelineApi; + + await page.getByTestId('more-actions').first().click(); + await page.waitForSelector('[data-testid="actions-dropdown"]', { + state: 'visible', + }); + + await page.waitForSelector( + '[data-testid="actions-dropdown"] [data-testid="edit-button"]', + { + state: 'visible', + } + ); + + await page + .getByTestId('actions-dropdown') + .getByTestId('edit-button') + .click(); + + await page.waitForLoadState('networkidle'); + await page.waitForSelector('[data-testid="loader"]', { + state: 'detached', + }); + await page.waitForSelector('[data-testid="select-all-test-cases"]', { + state: 'visible', + }); + + await expect(page.getByTestId('select-all-test-cases')).toHaveAttribute( + 'aria-checked', + 'true' + ); + }); + test('Non-owner user should not able to add test case', async ({ dataConsumerPage, dataStewardPage, diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.test.tsx index 38b6eb3e61f..b3113b03eff 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.test.tsx @@ -982,4 +982,55 @@ describe('TestCaseFormV1 Component', () => { }); }); }); + + describe('Test Cases Selection Logic', () => { + it('should set testCases to undefined when selectAllTestCases is not false', () => { + const testValues = { + selectAllTestCases: true, + otherField: 'value', + }; + + // Testing the new logic: values?.selectAllTestCases === false + const result = + testValues?.selectAllTestCases === false + ? ['testCase1', 'testCase2'] + : undefined; + + expect(result).toBeUndefined(); + }); + + it('should set testCases to array when selectAllTestCases is explicitly false', () => { + const testValues = { + selectAllTestCases: false, + otherField: 'value', + }; + + const mockSelectedTestCases = ['existingTestCase1', 'existingTestCase2']; + const mockCreatedTestCase = { name: 'newTestCase' }; + + // Testing the new logic: values?.selectAllTestCases === false + const result = + testValues?.selectAllTestCases === false + ? [mockCreatedTestCase.name, ...mockSelectedTestCases] + : undefined; + + expect(result).toEqual([ + 'newTestCase', + 'existingTestCase1', + 'existingTestCase2', + ]); + }); + + it('should set testCases to undefined when selectAllTestCases is undefined', () => { + const testValues: { selectAllTestCases?: boolean; otherField: string } = { + otherField: 'value', + }; + + // Testing the new logic: values?.selectAllTestCases === false + const result = + testValues?.selectAllTestCases === false ? ['testCase1'] : undefined; + + expect(result).toBeUndefined(); + }); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.tsx index 2f7b8e5cd89..b150f9df4f7 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataQuality/AddDataQualityTest/components/TestCaseFormV1.tsx @@ -796,9 +796,10 @@ const TestCaseFormV1: FC = ({ config: { type: ConfigType.TestSuite, entityFullyQualifiedName: testSuiteResponse.fullyQualifiedName, - testCases: values.selectAllTestCases - ? undefined - : [createdTestCase.name, ...selectedTestCases], + testCases: + values?.selectAllTestCases === false + ? [createdTestCase.name, ...selectedTestCases] + : undefined, }, }, }; diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/TableProfilerProvider.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/TableProfilerProvider.test.tsx index ad63d963621..573d63f5c30 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/TableProfilerProvider.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/TableProfilerProvider.test.tsx @@ -12,10 +12,14 @@ */ /* eslint-disable i18next/no-literal-string */ import { act, render, screen, waitFor } from '@testing-library/react'; +import { useContext } from 'react'; import { OperationPermission } from '../../../../context/PermissionProvider/PermissionProvider.interface'; import { MOCK_TABLE } from '../../../../mocks/TableData.mock'; import { getListTestCaseBySearch } from '../../../../rest/testAPI'; -import { TableProfilerProvider } from './TableProfilerProvider'; +import { + TableProfilerContext, + TableProfilerProvider, +} from './TableProfilerProvider'; // Mock dependencies jest.mock('../../../../hooks/useCustomLocation/useCustomLocation', () => { @@ -27,10 +31,18 @@ jest.mock('../../../../hooks/useCustomLocation/useCustomLocation', () => { jest.mock('../../../../context/TourProvider/TourProvider', () => ({ useTourProvider: jest.fn().mockReturnValue({ isTourOpen: false }), })); +// Create a mock for usePaging that tracks state changes +const mockPagingState = { total: 0 }; +const mockHandlePagingChange = jest.fn((newPaging) => { + mockPagingState.total = newPaging.total; +}); + jest.mock('../../../../hooks/paging/usePaging', () => ({ - usePaging: jest - .fn() - .mockReturnValue({ handlePagingChange: jest.fn(), pageSize: 10 }), + usePaging: jest.fn(() => ({ + handlePagingChange: mockHandlePagingChange, + pageSize: 10, + paging: mockPagingState, + })), })); jest.mock('../../../../rest/tableAPI', () => ({ getLatestTableProfileByFqn: jest.fn().mockResolvedValue({}), @@ -96,9 +108,31 @@ const mockPermissions = { ViewTests: true, } as OperationPermission; +// Test component to access context and display state +const TestComponent = () => { + const context = useContext(TableProfilerContext); + if (!context) { + return
No Context
; + } + + const { allTestCases, testCasePaging, table } = context; + + return ( +
+
{allTestCases.length}
+
{testCasePaging.paging?.total || 0}
+
+ {table?.testSuite?.name || 'no-test-suite'} +
+
+ ); +}; + describe('TableProfilerProvider', () => { beforeEach(() => { jest.clearAllMocks(); + // Reset mock paging state + mockPagingState.total = 0; }); it('renders children without crashing', async () => { @@ -165,4 +199,50 @@ describe('TableProfilerProvider', () => { }); }); }); + + describe('State Management for Test Cases', () => { + it('should initialize with empty test cases array and zero pagination', async () => { + const mockTable = { ...MOCK_TABLE, testSuite: undefined }; + + await act(async () => { + render( + + + + ); + }); + + // Wait for component to initialize + await waitFor(() => { + expect(screen.getByTestId('test-cases-count')).toBeInTheDocument(); + }); + + // Initial state should be empty + expect(screen.getByTestId('table-test-suite')).toHaveTextContent( + 'no-test-suite' + ); + expect(screen.getByTestId('test-cases-count')).toHaveTextContent('0'); + expect(screen.getByTestId('paging-total')).toHaveTextContent('0'); + }); + + it('should provide context values to consuming components', async () => { + await act(async () => { + render( + + + + ); + }); + + await waitFor(() => { + expect(screen.getByTestId('test-cases-count')).toBeInTheDocument(); + expect(screen.getByTestId('paging-total')).toBeInTheDocument(); + expect(screen.getByTestId('table-test-suite')).toBeInTheDocument(); + }); + }); + }); }); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/TableProfilerProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/TableProfilerProvider.tsx index 3e9fa8c5f98..3a04ed5a3b2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/TableProfilerProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Database/Profiler/TableProfiler/TableProfilerProvider.tsx @@ -67,7 +67,7 @@ export const TableProfilerContext = export const TableProfilerProvider = ({ children, permissions, - table, + table: tableEntity, testCaseSummary, }: TableProfilerProviderProps) => { const { t } = useTranslation(); @@ -87,6 +87,7 @@ export const TableProfilerProvider = ({ useState(DEFAULT_RANGE_DATA); const [isTestCaseDrawerOpen, setIsTestCaseDrawerOpen] = useState(false); const [testLevel, setTestLevel] = useState(); + const [table, setTable] = useState(tableEntity); const isTableDeleted = useMemo(() => table?.deleted, [table]); @@ -208,6 +209,22 @@ export const TableProfilerProvider = ({ }; const onTestCaseSubmit = (testCase: TestCase) => { + setTable((prev) => { + if (!prev) { + return prev; + } + + return { ...prev, testSuite: testCase.testSuite }; + }); + + const paging = { + ...testCasePaging.paging, + // Increase total count as we are adding new test case + total: (testCasePaging.paging?.total || 0) + 1, + }; + + testCasePaging.handlePagingChange(paging); + setAllTestCases((prevTestCases) => { return [testCase, ...prevTestCases]; });