[BI Data Mapper] Add Level 01 E2E test #653
Conversation
…tensions-public into bi-dm-e2e-test
|
Warning Rate limit exceeded@KCSAbeywickrama has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 28 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds end-to-end Playwright test infrastructure and test data for the BI Data Mapper, minor UI test-id and styling tweaks, and small test-integration helpers/exposures to enable automated tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx (1)
176-178: Consider label clarity for user understanding.The menu item ID change from
"a2a-direct"to"direct"simplifies the identifier, which is good. However, the label change from "Map Input Array to Output Array" to "Map Anyway" may be less clear for users trying to understand what this option does.Consider whether "Map Anyway" provides sufficient context, or if a more descriptive label like "Map Directly" or retaining the original would better communicate the action to users.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/utils.ts (1)
71-71: Consider consolidating the default project name.The changes correctly align the project directory name from
testProjecttosampleacross initialization paths. However, there's a maintainability concern: whileinitTestaccepts an optionalprojectNameparameter, several functions hardcode'sample':
resumeVSCode(line 71)createProject's locator (line 194:text=sample)verifyGeneratedSource(line 316:path.join(newProjectPath, 'sample', ...))This creates partial support for custom project names that won't function end-to-end.
Suggestions:
If custom project names aren't needed, consider removing the
projectNameparameter frominitTestto clarify that only'sample'is supported.If custom project names should be supported, pass
projectNamethrough the call chain and use it consistently inresumeVSCode,verifyGeneratedSource, and thecreateProjectlocator.For maintainability, extract
'sample'as a module-level constant:const DEFAULT_PROJECT_NAME = 'sample';For the current use case (default
'sample'project), the implementation works correctly.Also applies to: 201-201, 234-234
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/ProjectExplorer.ts (1)
62-66: Consider waiting for refresh completion.The
refreshmethod clicks the Refresh button but doesn't wait for the operation to complete. This could lead to timing issues in tests if subsequent operations expect the refresh to be finished.Consider adding a wait for the refresh operation to complete:
public async refresh(projectName: string) { await this.page.getByRole('treeitem', { name: projectName }).hover(); const refreshBtn = this.page.getByRole('button', { name: 'Refresh' }); await refreshBtn.click(); + // Wait for refresh to complete - adjust selector as needed + await this.page.waitForTimeout(500); }Note: The specific wait mechanism should match your application's behavior (e.g., waiting for a loading indicator to disappear, or for specific elements to be updated).
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (4)
45-52: Scrolling retry loop could fail silently.The scrolling retry loop has a hardcoded maximum of 5 iterations. If the element is still not clickable after 5 attempts, the method completes without indication of failure, potentially causing subsequent operations to fail.
Consider adding error handling or logging:
public async scrollOutputUntilClickable(locator: Locator) { const outputNode = this.webView.locator(`div[data-testid$="Output-node"]`); await outputNode.hover(); - for (let i = 0; !(await this.isClickable(locator)) && i < 5; i++) { + const maxAttempts = 5; + for (let i = 0; i < maxAttempts; i++) { + if (await this.isClickable(locator)) { + return; + } await page.page.mouse.wheel(0, 400); } + // Log warning if element is still not clickable + console.warn('Element may not be clickable after scrolling attempts'); }
142-146: Synchronous file I/O blocks event loop in async function.Line 145 uses
fs.writeFileSyncwhich blocks the Node.js event loop. In an async context, prefer async file operations.Use async file operations:
+import { promises as fsPromises } from 'fs'; + public async saveSnapshot(snapshotFile: string) { const root = this.webView.locator(`div#data-mapper-canvas-container`); await root.waitFor(); - fs.writeFileSync(snapshotFile, await root.innerHTML()); + await fsPromises.writeFile(snapshotFile, await root.innerHTML()); }
158-168: Synchronous file I/O blocks event loop.Both
updateProjectFileSyncandupdateDataFileSyncuse synchronous file operations (fs.readFileSync,fs.writeFileSync) which block the Node.js event loop.Consider creating async versions:
+export async function updateProjectFile(sourceFile: string, targetFile: string) { + const sourcePath = path.join(dmDataDir, sourceFile); + const targetPath = path.join(newProjectPath, 'sample', targetFile); + await fsPromises.writeFile(targetPath, await fsPromises.readFile(sourcePath, 'utf8')); +} + +export async function updateDataFile(sourceFile: string, targetFile: string) { + const sourcePath = path.join(newProjectPath, 'sample', sourceFile); + const targetPath = path.join(dmDataDir, targetFile); + await fsPromises.writeFile(targetPath, await fsPromises.readFile(sourcePath, 'utf8')); +}
185-190: Synchronous file I/O blocks event loop.
compareFilesSyncusesfs.readFileSyncwhich blocks the event loop.Consider creating an async version:
+export async function compareFiles(file1: string, file2: string) { + const file1Content = (await fsPromises.readFile(file1, 'utf8')).replaceAll('\r\n', '\n'); + const file2Content = (await fsPromises.readFile(file2, 'utf8')).replaceAll('\r\n', '\n'); + return file1Content === file2Content; +}workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/inline/map1.bal.txt (1)
1-11: Code is functionally correct, but consider breaking line 6 for readability.Line 6 contains a complex 7-field mapping with nested objects and multiple arithmetic expressions on a single line, making it difficult to debug and maintain. For test data, this is acceptable as-is, but splitting the output object construction across multiple lines would improve clarity.
OutRoot output = { - oPrimDirect: input.iPrimDirect, oPrimDirectErr: input.iPrimDirectErr, oManyOne: input.iManyOne1 + input.iManyOne2 + input.iManyOne3, oManyOneErr: input.iManyOneErr1 + input.iPrimDirectErr + input.iManyOneErr2, oObjDirect: input.iObjDirect, oObjDirectErr: input.iObjDirect, oObjProp: {p1: input.iObjDirect.d1, p2: input.iObjProp.op2} + oPrimDirect: input.iPrimDirect, + oPrimDirectErr: input.iPrimDirectErr, + oManyOne: input.iManyOne1 + input.iManyOne2 + input.iManyOne3, + oManyOneErr: input.iManyOneErr1 + input.iPrimDirectErr + input.iManyOneErr2, + oObjDirect: input.iObjDirect, + oObjDirectErr: input.iObjDirect, + oObjProp: {p1: input.iObjDirect.d1, p2: input.iObjProp.op2} };workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts (1)
29-29: Consider replacing empty object pattern with underscore.The empty object pattern
async ({ }, testInfo)is flagged by Biome but is valid Playwright syntax. For cleaner code, consider using an underscore to indicate the unused fixtures parameter.Apply this pattern to all test functions (lines 29, 75, 99, 123):
-test('Create', async ({ }, testInfo) => { +test('Create', async (_, testInfo) => {workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts (1)
31-31: Consider replacing empty object pattern with underscore.Same as the reusable data mapper spec, the empty object pattern
async ({ }, testInfo)can be replaced with an underscore for cleaner code and to satisfy the linter.Apply this pattern to all test functions (lines 31, 82, 110, 138):
-test('Create', async ({ }, testInfo) => { +test('Create', async (_, testInfo) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (61)
workspaces/ballerina/data-mapper/src/components/DataMapper/Header/DataMapperHeader.tsx(1 hunks)workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx(2 hunks)workspaces/ballerina/data-mapper/src/components/Diagram/Node/LinkConnector/LinkConnectorNodeWidget.tsx(1 hunks)workspaces/ballerina/data-mapper/src/components/Diagram/Node/QueryExprConnector/QueryExprConnectorNodeWidget.tsx(1 hunks)workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/Tree/Tree.tsx(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/ProjectExplorer.ts(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/del1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/del2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/del3.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/del4.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/init.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map3.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/del1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/del2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/del3.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/del4.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/init.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/map1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/map2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/map3.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/types.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del3.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del4.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del5.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/init.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/map1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/map2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/map3.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del3.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del4.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del5.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/init.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/map1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/map2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/map3.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/types.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/inline/del1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/inline/del2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/inline/init.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/inline/map1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/inline/map2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/reusable/del1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/reusable/del2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/reusable/init.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/reusable/map1.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/reusable/map2.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/types.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/create/inline/final.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/create/inline/init.bal.txt(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/test.list.ts(2 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/utils.ts(3 hunks)workspaces/bi/bi-extension/tsconfig.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-05T10:31:47.583Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 830
File: workspaces/ballerina/ballerina-extension/test/ai/post_proccess/post.test.ts:0-0
Timestamp: 2025-11-05T10:31:47.583Z
Learning: In the workspaces/ballerina/ballerina-extension project, the tsconfig.json uses "rootDirs": ["src", "test"], which allows test files to import from src using shorter relative paths. For example, from test/ai/post_proccess/, the import '../../stateMachine' correctly resolves to src/stateMachine.ts due to this configuration.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/utils.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/test.list.tsworkspaces/bi/bi-extension/tsconfig.json
📚 Learning: 2025-11-10T15:05:11.309Z
Learnt from: madushajg
Repo: wso2/vscode-extensions PR: 868
File: workspaces/bi/bi-extension/src/utils.ts:224-242
Timestamp: 2025-11-10T15:05:11.309Z
Learning: The workspaces/bi/bi-extension and workspaces/ballerina/ballerina-extension are separate VS Code extensions that are packaged and distributed independently, so they cannot share code via imports and must maintain their own implementations of common utilities.
Applied to files:
workspaces/bi/bi-extension/tsconfig.json
🧬 Code graph analysis (2)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts (1)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts (1)
createTests(26-166)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts (2)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts (1)
createTests(24-147)workspaces/mi/mi-extension/src/test/e2e-playwright-tests/components/ProjectExplorer.ts (1)
ProjectExplorer(21-121)
🪛 Biome (2.1.2)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts
[error] 29-29: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 75-75: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 99-99: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 123-123: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts
[error] 31-31: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 82-82: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 110-110: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 138-138: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (54)
workspaces/bi/bi-extension/tsconfig.json (1)
6-9: Verify DOM types are needed for the E2E test infrastructure.Adding the
DOMlibrary will provide type definitions for browser APIs likeDocument,Window, andElement. However, since Playwright tests execute in a Node.js environment rather than a browser, and Playwright's own types should provide necessary bindings, please verify that test code actually references DOM APIs that require this addition.An alternative approach would be to create a separate
tsconfigfor theui-testdirectory if only E2E tests need DOM types, keeping the main config focused on the extension source code.workspaces/ballerina/data-mapper/src/components/Diagram/Node/commons/Tree/Tree.tsx (1)
72-74: LGTM! Selection outline enhances UI feedback.The conditional outline style provides clear visual feedback when a tree header is selected, improving the user experience during E2E testing and normal usage.
workspaces/ballerina/data-mapper/src/components/DataMapper/Header/DataMapperHeader.tsx (1)
55-55: LGTM! Test ID addition supports E2E automation.The
data-testidattribute enables stable test selectors for the back button in automated tests.workspaces/ballerina/data-mapper/src/components/Diagram/Node/LinkConnector/LinkConnectorNodeWidget.tsx (1)
69-69: LGTM! Test ID now uses port name with proper null-safety.The use of optional chaining (
node?.targetPort?.getName()) ensures the test ID generation handles cases where the target port may not exist.workspaces/ballerina/data-mapper/src/components/Diagram/Node/QueryExprConnector/QueryExprConnectorNodeWidget.tsx (1)
86-86: LGTM! Test ID update aligns with LinkConnectorNodeWidget.The test ID now uses
node?.targetPort?.getName(), consistent with the approach in LinkConnectorNodeWidget and properly handling null-safety with optional chaining.workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx (1)
192-193: LGTM! Menu item identifier simplification improves maintainability.The simplified IDs (
"custom-func"and"transform-func") are more concise while the new labels clearly describe the mapping operations to users.Also applies to: 198-199
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/init.bal.txt (1)
1-1: LGTM! Test data is appropriate for initialization/deletion scenarios.This test data file correctly represents an empty state for array-root reusable mapper tests.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (3)
195-318: Test orchestration function is well-structured.The
testBasicMappingsfunction provides comprehensive test coverage with clear step-by-step validation. The console.log statements are helpful for test debugging and progress tracking.
320-456: Test orchestration function is well-structured.The
testArrayInnerMappingsfunction provides thorough coverage of array mapping scenarios with good organization and clear test steps.
458-595: Test orchestration function is well-structured.The
testArrayRootMappingsfunction provides comprehensive coverage of root array mapping scenarios with detailed step-by-step validation.workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del5.bal.txt (1)
1-1: LGTM! Test data is appropriate.This test data file correctly represents an empty deletion state (del5) for array-root reusable mapper tests.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/init.bal.txt (1)
1-11: LGTM! Appropriate test data for inline data mapper initialization.The function demonstrates proper error handling with do-on-fail pattern and appropriate logging. The initialization of unused variables is expected for test data representing an initial state.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/reusable/map2.bal.txt (1)
1-1: LGTM! Test data represents direct mapping scenario.This function returns the input directly, which is appropriate for testing root-level direct mapping. Type compatibility between
InRootandOutRootwill be validated by the Ballerina compiler during test execution.workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/del2.bal.txt (1)
1-4: LGTM! Query expression with empty projection is appropriate.The query expression maps
input.iArr1Dwith an empty projection (select {}), which is suitable for testing deletion scenarios where field mappings have been removed but the array structure remains.workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/del4.bal.txt (1)
1-3: LGTM! Empty function body is appropriate for deletion test.The empty function body represents a complete deletion state, which is suitable for the del4 test scenario where all mappings have been removed.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/del4.bal.txt (1)
1-13: Consistent deletion test scaffolding.Follows the standard inline test pattern with proper error handling. The empty output object aligns with expected teardown/deletion scenario semantics.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/del3.bal.txt (1)
1-3: Verify partial object initialization aligns with type definition.Line 2 initializes oArr1D array with
{p2: ""}(p1 field omitted). Confirm this partial object initialization is valid against the OutArrType definition in types.bal.txt, especially if p1 is a required field.Verify the type definition for OutArrType in workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/types.bal.txt to confirm whether p1 is optional, nullable, or has a default value that allows this initialization pattern.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del5.bal.txt (1)
1-11: Consistent root-array deletion scaffolding.Properly initializes empty root arrays for deletion test scenario. Follows established inline error-handling pattern and aligns with array-inner deletion tests.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/del1.bal.txt (1)
1-4: Query expression array projection is well-formed.Uses standard Ballerina query syntax to project input.iArr1D to empty objects. Appropriate for testing array traversal within query expressions, aligning with PR objectives for "query-expression workflows."
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/init.bal.txt (1)
1-11: Root array initialization setup is properly structured.Consistent with array initialization patterns. Empty arrays provide clean starting state for array-based test scenarios, aligning with PR objectives for array-root workflows.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/map3.bal.txt (1)
1-11: Root-array-to-nested-array mapping demonstrates expected behavior.Properly maps root-level InRoot[] input to oArr1D fields within multiple OutRoot[] elements. Aligns with PR objectives for root array mapping scenarios and demonstrates nested array field population.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/create/inline/init.bal.txt (1)
1-9: Initialization template structure is sound.The empty do block follows the standard inline pattern and appears to serve as a template or setup placeholder. Confirm that execution of this function (with an empty do block) meets the test initialization requirements and won't cause test failures.
If the empty do block is intentional as a minimal initialization fixture, this is fine. If it should contain setup logic, please populate accordingly.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/map1.bal.txt (1)
1-5: Mapping logic is sound and follows query expression patterns correctly.The nested query expressions correctly transform input array elements to output array elements with computed fields. This aligns with the array-root mapping test scenario.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/inline/map2.bal.txt (1)
1-11: Error handling structure is correct; verify mapping initialization intent.The do-on fail pattern properly handles errors. However, line 6 uses direct assignment
output = inputrather than constructing OutRoot with mapped fields. Confirm this represents the expected state for a basic inline mapping test fixture.workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del4.bal.txt (1)
1-11: Empty state fixture is correctly structured for deletion test scenarios.The empty array initialization properly represents the state after delete operations for array-root mappings. Error handling is included as expected.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/map2.bal.txt (1)
1-4: Record literal syntax and identity projection are correctly implemented.The arrow function correctly projects the input array field as an identity mapping to the output. This appropriately represents array-inner test scenarios.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/map3.bal.txt (1)
1-3: Mixed element types in array literal—verify type compatibility.The array combines object literals and a scalar value. Ensure that the OutRoot.oArr1D field type supports this mixed-type array (e.g., defined as
object | int).workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del1.bal.txt (1)
1-15: Nested empty projections correctly represent intermediate deletion state.The from-select pattern with empty object projection
select {}appropriately represents array-root mappings after field deletion. Proper error handling is included.workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del3.bal.txt (1)
1-4: Arrow function with empty projection correctly represents cleared mappings.The from-select pattern with empty object projection appropriately models the state after deleting mapped fields in array-root reusable mappings.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/del2.bal.txt (1)
1-14: Inline array field mapping with empty projections is correctly structured.The from-select pattern with empty projections appropriately represents the deletion state for array-inner inline mappings. Error handling follows established patterns.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/inline/del2.bal.txt (1)
1-11: LGTM!The test data structure is correct, with proper error handling for the deletion scenario.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del1.bal.txt (1)
1-5: LGTM!The reusable function properly maps nested arrays with the correct signature and empty projection pattern for the deletion test case.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/del1.bal.txt (1)
1-14: LGTM!The inline array projection with empty select is correct, and error handling follows the established pattern.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/reusable/map1.bal.txt (1)
1-4: LGTM!The field projection logic is sound (p1 computed as sum, p2 as direct copy), with clean and readable syntax.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/del3.bal.txt (1)
1-13: LGTM!The array initialization with sample data and error handling follows the established pattern correctly.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/reusable/init.bal.txt (1)
1-9: LGTM!The initialization function with default/empty values is correct for the init test scenario, and the arrow notation is clean and appropriate.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map1.bal.txt (1)
1-14: LGTM!The array field projection with computed and direct-copy fields is correct, with clear logic (p1 = sum, p2 = copy) and proper error handling.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del2.bal.txt (2)
1-15: AI summary inconsistency detected.The AI summary states the function signature is
public function output(InRoot[] input) returns OutRoot[], but the actual code definespublic function main() returns error?. The actual code is correct for inline test data and follows the expected pattern with error handling.
3-14: Test data structure is correct.The test data correctly demonstrates a deletion scenario with proper error handling. The empty select block at line 9 and the do/on fail pattern are appropriate for this E2E test fixture.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/map1.bal.txt (1)
3-14: LGTM!The test data correctly demonstrates array-root mapping with field transformations. The arithmetic operation and field mappings are properly structured for E2E testing.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map2.bal.txt (1)
3-13: LGTM!The test data correctly demonstrates the array-inner mapping scenario where the root is a record type containing an array field. The passthrough mapping pattern is appropriate.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/map2.bal.txt (1)
1-5: LGTM!The reusable data mapper variant correctly uses arrow function syntax and omits error handling, which is appropriate for reusable functions. The mapping logic is equivalent to the inline variant.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/del3.bal.txt (1)
3-13: LGTM!The test data correctly demonstrates a complete deletion scenario with an entirely empty select block. This is appropriate for testing the removal of all field mappings in the data mapper.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/inline/map2.bal.txt (2)
1-15: AI summary file path is incorrect.The AI summary references
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map2.bal.txt, but the actual file path isarray-root/inline/map2.bal.txt. The code itself is correct and demonstrates the array-root scenario.
3-14: LGTM!The test data correctly demonstrates array-root mapping with passthrough of array elements. The structure and error handling are appropriate for E2E testing.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/test.list.ts (2)
54-55: LGTM!The new data-mapper test imports follow the existing pattern and are correctly structured.
68-107: Verify that disabling all other tests is intentional.All existing E2E test suites (automation, API services, event integrations, file integrations, other artifacts, etc.) have been commented out, leaving only the new data-mapper tests active. This means CI will exclusively run data-mapper tests.
Is this change intended to remain in the merged code, or should the other tests be re-enabled before merging?
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/types.bal.txt (1)
1-21: LGTM!The type definitions are well-structured for array-root test scenarios. Using closed records ensures strict type checking, which is appropriate for test fixtures.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-root/reusable/del2.bal.txt (1)
1-5: LGTM! Test data is well-structured for deletion scenarios.The empty object in the inner select (line 4) is intentionally designed to test deletion of array element mappings, aligning with the PR's objective to validate array-related workflows including deletion operations.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/reusable/map1.bal.txt (1)
1-9: LGTM! Mapping function covers diverse test scenarios.The function appropriately tests various mapping patterns including direct mappings, nested object mappings, and many-to-one field aggregations. The fields with "Err" suffix (lines 2, 6, 7) are intentionally designed to test type mismatches and error handling behavior.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts (1)
24-146: Excellent test coverage and structure!The test suite comprehensively covers the reusable data mapper workflows as outlined in the PR objectives:
- Create workflow validates data mapper creation and initialization
- Basic test covers field mapping and deletion operations
- Array Inner test validates array element operations and query expressions
- Array Root test covers root-level array mapping scenarios
The test organization, error handling, and use of shared utilities (FileUtils, TestScenarios) demonstrate good testing practices.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts (1)
26-166: Excellent inline data mapper test coverage!The test suite provides comprehensive coverage of inline data mapper workflows:
- Create test validates the full flow from adding a Declare Variable node to opening the data mapper and navigating back
- Basic, Array Inner, and Array Root tests thoroughly exercise mapping operations aligned with PR objectives
- Proper integration with Diagram component for node manipulation
- Good use of conditional logic to handle already-open data mapper scenarios
The parallel structure with the reusable data mapper tests enhances maintainability and test consistency.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/basic/types.bal.txt (1)
1-46: LGTM! Type definitions are well-designed for comprehensive test coverage.The type definitions effectively support various test scenarios:
- Primitive field mappings (iPrimDirect → oPrimDirect)
- Nested object mappings (iObjDirect, iObjProp → oObjDirect, oObjProp)
- Many-to-one field aggregations (iManyOne1/2/3)
- Intentional type mismatches for error testing (fields with "Err" suffix)
The structure provides a solid foundation for testing both successful and error-handling mapping behaviors.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/types.bal.txt (1)
1-21: LGTM! Type definitions appropriately structured for array mapping tests.The type definitions effectively support array-inner test scenarios:
- InRoot combines array fields with additional properties to test context-aware array mappings
- InArrType → OutArrType mapping allows testing selective field mapping (3 fields → 2 fields)
- Structure aligns with the PR's objectives for array element operations, query expressions, and focused view navigation
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
Show resolved
Hide resolved
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
Show resolved
Hide resolved
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
Show resolved
Hide resolved
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
Show resolved
Hide resolved
...-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map3.bal.txt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (4)
105-110: Empty catch blocks swallow errors.This issue was flagged in a previous review. The empty catch blocks at lines 107 and 110 silently swallow errors, making debugging difficult when progress ring state checks fail.
128-134: Potential negative index if breadcrumbs count is 0.This issue was flagged in a previous review. Line 130 computes
await breadcrumbs.count() - 1which could be -1 if there are no breadcrumbs, leading to unexpected behavior.
136-141: Potential negative index if breadcrumbs count is 0.This issue was flagged in a previous review. Line 138 has the same potential negative index issue.
173-178: Remove commented-out debugging code.This issue was flagged in a previous review. Lines 173-178 contain commented-out debugging code that should be removed before merging.
🧹 Nitpick comments (4)
workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx (1)
578-585: Stale closure risk with empty dependency array.
onDMRefreshreferencesrpcClientandrefreshDMModel, but this effect has an empty dependency array. If these values change during the component lifecycle,window.__refreshDMwill hold a stale reference.Consider adding the dependency or wrapping
onDMRefreshinuseCallback:useEffect(() => { // Expose function to refresh data mapper in playwright tests (window as any).__refreshDM = onDMRefresh; return () => { delete (window as any).__refreshDM; }; - }, []); + }, [onDMRefresh]);This also requires memoizing
onDMRefreshwithuseCallbackto prevent unnecessary reassignments on each render.workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts (2)
31-31: Empty object pattern flagged by static analysis.The empty destructuring pattern
{ }is flagged by Biome. While this is a common Playwright pattern to accesstestInfowithout using fixtures, you can silence the linter by using an underscore:- test('Create', async ({ }, testInfo) => { + test('Create', async ({}, testInfo) => {Or if linting still complains, use a suppression comment. The same applies to lines 82, 110, and 138.
90-103: Consider extracting repeated webView setup logic.The pattern of switching to iframe, checking if Data Mapper is open, and navigating to open it is duplicated across Basic, Array Inner, and Array Root tests. This could be extracted into a helper function for maintainability:
async function ensureDataMapperOpen(webView: Frame, outputText: string) { const isDataMapperOpened = await webView.getByRole('heading', { name: 'Data Mapper' }).isVisible(); if (!isDataMapperOpened) { await webView.getByRole('heading', { name: 'sample' }).waitFor(); await page.page.getByRole('treeitem', { name: 'main' }).click(); await webView.getByRole('heading', { name: 'Automation' }).waitFor(); await webView.getByText(outputText).click(); await webView.getByRole('button', { name: 'Open in Data Mapper' }).click({ force: true }); } return isDataMapperOpened; }This would reduce duplication and make the tests more maintainable.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (1)
66-68: Consider adding explicit timeout to waitForProgressEnd.The
waitForProgressEndmethod relies on Playwright's default timeout. For clarity and to prevent indefinite waits during test failures, consider adding an explicit timeout:public async waitForProgressEnd() { - await this.webView.waitForSelector('vscode-progress-ring', { state: 'detached' }); + await this.webView.waitForSelector('vscode-progress-ring', { state: 'detached', timeout: 30000 }); }This makes the expected wait time explicit and helps with debugging timeout issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx(1 hunks)workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx(2 hunks)workspaces/ballerina/data-mapper/src/components/Diagram/Node/QueryExprConnector/QueryExprConnectorNodeWidget.tsx(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/ballerina/data-mapper/src/components/Diagram/Node/QueryExprConnector/QueryExprConnectorNodeWidget.tsx
- workspaces/ballerina/data-mapper/src/components/Diagram/Label/MappingOptionsWidget.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.test.{ts,tsx} : Create snapshot tests for all node widgets and verify visual consistency across renders
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/stories/**/*.json : Provide sample data files in JSON format for Storybook stories covering: empty project, automation only, simple service, multiple services, complex connections, AI services, GraphQL services, and multiple connections
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/**/*.config.{js,ts} : Use minimatch-compatible glob patterns for file matching in build and test configuration files
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.test.{ts,tsx} : Create snapshot tests for all node widgets and verify visual consistency across renders
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.tsworkspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsxworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/stories/**/*.json : Provide sample data files in JSON format for Storybook stories covering: empty project, automation only, simple service, multiple services, complex connections, AI services, GraphQL services, and multiple connections
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts
📚 Learning: 2025-11-20T11:04:33.712Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 898
File: workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx:38-38
Timestamp: 2025-11-20T11:04:33.712Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/SidePanel/QueryClauses/ClausesPanel.tsx, the use of `useDMQueryClausesPanelStore.getState()` to access `clauseToAdd` and `setClauseToAdd` (instead of the hook subscription pattern) is intentional to prevent re-renders when these state values change.
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/components/**/*.tsx : Use React 18.2.0 features including concurrent rendering and automatic batching; avoid class components in favor of functional components with hooks
Applied to files:
workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx
📚 Learning: 2025-11-24T14:51:49.267Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 998
File: workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx:113-132
Timestamp: 2025-11-24T14:51:49.267Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx, if `textFieldRef.current` is not undefined, `textFieldRef.current.inputElement` is guaranteed to exist. If `inputElement` doesn't exist when `current` exists, it's a fatal error that should reach the error boundary rather than being handled with defensive null checks.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
📚 Learning: 2025-11-20T06:44:07.498Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 969
File: workspaces/ballerina/data-mapper/src/components/Diagram/utils/node-utils.ts:23-44
Timestamp: 2025-11-20T06:44:07.498Z
Learning: In the data mapper node-utils.ts, when `lastViewIndex == 0`, the code is at the root view and should NOT use `focusInputRootMap` for fallback input node resolution. The truthiness check on `lastViewIndex` is intentional to skip the fallback at the root level.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
🧬 Code graph analysis (2)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/Utils.ts (2)
initTest(179-220)page(39-39)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/Utils.ts (1)
page(39-39)
🪛 Biome (2.1.2)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts
[error] 31-31: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 82-82: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 110-110: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 138-138: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (3)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (3)
194-319: Comprehensive test scenario implementation.The
testBasicMappingsfunction provides thorough coverage of:
- Direct mapping with and without errors
- Many-to-one mappings
- Object direct and property mappings
- Deletion workflows
- Root field mappings
Good use of descriptive logging and file content verification at each stage.
321-457: Well-structured array inner mappings test.The
testArrayInnerMappingsfunction covers:
- Input/output preview
- Query expression mappings
- Focused view navigation (breadcrumb and back button)
- Array initialization and element addition
- Element mapping and deletion
The test flow aligns well with the PR objectives for array-related workflows.
459-595: Thorough array root mappings test.The
testArrayRootMappingsfunction provides excellent coverage of:
- Nested query expressions
- Inner focused view operations
- Root and intermediate mapping deletions
- Root array element management
This completes the Level 01 E2E test coverage as specified in the linked issue.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts
Outdated
Show resolved
Hide resolved
…e data mapper tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (3)
40-52: Consider adding a maximum scroll attempt check failure.The
scrollOutputUntilClickablemethod silently stops after 5 attempts without indicating whether the element became clickable. If the element remains unclickable after all attempts, subsequentclick()inscrollClickOutputmay fail with a less informative error.public async scrollOutputUntilClickable(locator: Locator) { const outputNode = this.webView.locator(`div[data-testid$="Output-node"]`); await outputNode.hover(); - for (let i = 0; !(await this.isClickable(locator)) && i < 5; i++) { + let isClickable = false; + for (let i = 0; i < 5; i++) { + if (await this.isClickable(locator)) { + isClickable = true; + break; + } await page.page.mouse.wheel(0, 400); } + if (!isClickable && !(await this.isClickable(locator))) { + console.warn('Element may not be fully clickable after scrolling'); + } }
143-147: Potential race condition insaveSnapshot.The
saveSnapshotmethod uses synchronousfs.writeFileSyncafter awaiting the locator. This is generally fine for tests, but the innerHTML could theoretically change betweenwaitForandinnerHTML()calls under rapid UI updates.For robustness, consider capturing innerHTML immediately after waitFor:
public async saveSnapshot(snapshotFile: string) { const root = this.webView.locator(`div#data-mapper-canvas-container`); await root.waitFor(); - fs.writeFileSync(snapshotFile, await root.innerHTML()); + const content = await root.innerHTML(); + fs.writeFileSync(snapshotFile, content); }
159-169: File operations lack error handling.Both
updateProjectFileSyncandupdateDataFileSyncwill throw if files don't exist or paths are invalid. Consider whether wrapping in try-catch with descriptive errors would improve debugging in CI.export function updateProjectFileSync(sourceFile: string, targetFile: string) { const sourcePath = path.join(dmDataDir, sourceFile); const targetPath = path.join(newProjectPath, 'sample', targetFile); + if (!fs.existsSync(sourcePath)) { + throw new Error(`Source file not found: ${sourcePath}`); + } fs.writeFileSync(targetPath, fs.readFileSync(sourcePath, 'utf8')); }workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts (1)
29-31: Empty object pattern in test callback.Biome flags the empty destructuring pattern
{ }. While Playwright allows this, the pattern can be replaced with an underscore or simply omit the first parameter if no fixtures are used.- test('Create', async ({ }, testInfo) => { + test('Create', async (_fixtures, testInfo) => {Apply similarly to lines 82, 110, and 138.
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts (1)
29-29: Empty object pattern in test callback.Same issue as inline spec - Biome flags the empty destructuring pattern
{ }. Consider using_fixturesor omitting if no fixtures are needed.- test('Create', async ({ }, testInfo) => { + test('Create', async (_fixtures, testInfo) => {Apply similarly to lines 75, 99, and 123.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx(1 hunks)workspaces/ballerina/data-mapper/src/components/DataMapper/Header/DataMapperHeader.tsx(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts(1 hunks)workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- workspaces/ballerina/data-mapper/src/components/DataMapper/Header/DataMapperHeader.tsx
- workspaces/ballerina/ballerina-visualizer/src/views/DataMapper/DataMapperView.tsx
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map3.bal.txt:6-8
Timestamp: 2025-11-26T06:37:07.886Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/ directories, BAL test data files (such as those in array-inner/inline/, array-root/inline/, basic/inline/, and their reusable counterparts) may intentionally contain type errors and other violations. These are comparison files used to test data mapper functionality and error handling, so such errors should not be flagged as issues.
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:173-178
Timestamp: 2025-11-26T06:35:19.217Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the commented-out debugging block in the verifyFileContent function (lines 172-177 containing console.log, page.pause, and updateDataFileSync) is intentionally kept as a developer utility for updating test data files when needed. It should not be removed.
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/stories/**/*.json : Provide sample data files in JSON format for Storybook stories covering: empty project, automation only, simple service, multiple services, complex connections, AI services, GraphQL services, and multiple connections
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:86-112
Timestamp: 2025-11-26T06:33:22.950Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, empty catch blocks around progress ring waitForSelector calls (state: 'attached' and 'detached') are intentional. The progress ring duration depends on machine performance and may appear very briefly, causing the wait to miss the event. The try-catch allows the test to proceed gracefully regardless of timing.
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.test.{ts,tsx} : Create snapshot tests for all node widgets and verify visual consistency across renders
📚 Learning: 2025-11-26T06:35:19.217Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:173-178
Timestamp: 2025-11-26T06:35:19.217Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the commented-out debugging block in the verifyFileContent function (lines 172-177 containing console.log, page.pause, and updateDataFileSync) is intentionally kept as a developer utility for updating test data files when needed. It should not be removed.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts
📚 Learning: 2025-11-26T06:37:07.886Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/array-inner/inline/map3.bal.txt:6-8
Timestamp: 2025-11-26T06:37:07.886Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/dm-data/ directories, BAL test data files (such as those in array-inner/inline/, array-root/inline/, basic/inline/, and their reusable counterparts) may intentionally contain type errors and other violations. These are comparison files used to test data mapper functionality and error handling, so such errors should not be flagged as issues.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/**/*.test.{ts,tsx} : Create snapshot tests for all node widgets and verify visual consistency across renders
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts
📚 Learning: 2025-11-26T06:34:09.752Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:128-134
Timestamp: 2025-11-26T06:34:09.752Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBreadcrumb() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts
📚 Learning: 2025-11-26T06:33:22.950Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:86-112
Timestamp: 2025-11-26T06:33:22.950Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, empty catch blocks around progress ring waitForSelector calls (state: 'attached' and 'detached') are intentional. The progress ring duration depends on machine performance and may appear very briefly, causing the wait to miss the event. The try-catch allows the test to proceed gracefully regardless of timing.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts
📚 Learning: 2025-11-26T07:49:56.428Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 653
File: workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts:136-141
Timestamp: 2025-11-26T07:49:56.428Z
Learning: In workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts, the goPrevViewBackButton() method is only called when in a focused view, ensuring breadcrumbs are always present. No guard for empty breadcrumbs is needed.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.tsworkspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Applies to workspaces/ballerina/component-diagram/src/stories/**/*.json : Provide sample data files in JSON format for Storybook stories covering: empty project, automation only, simple service, multiple services, complex connections, AI services, GraphQL services, and multiple connections
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts
📚 Learning: 2025-11-24T14:51:49.267Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 998
File: workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx:113-132
Timestamp: 2025-11-24T14:51:49.267Z
Learning: In workspaces/ballerina/data-mapper/src/components/DataMapper/Header/ExpressionBar.tsx, if `textFieldRef.current` is not undefined, `textFieldRef.current.inputElement` is guaranteed to exist. If `inputElement` doesn't exist when `current` exists, it's a fatal error that should reach the error boundary rather than being handled with defensive null checks.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
📚 Learning: 2025-11-20T06:44:07.498Z
Learnt from: KCSAbeywickrama
Repo: wso2/vscode-extensions PR: 969
File: workspaces/ballerina/data-mapper/src/components/Diagram/utils/node-utils.ts:23-44
Timestamp: 2025-11-20T06:44:07.498Z
Learning: In the data mapper node-utils.ts, when `lastViewIndex == 0`, the code is at the root view and should NOT use `focusInputRootMap` for fallback input node resolution. The truthiness check on `lastViewIndex` is intentional to skip the fallback at the root level.
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
📚 Learning: 2025-11-25T06:34:10.812Z
Learnt from: CR
Repo: wso2/vscode-extensions PR: 0
File: workspaces/ballerina/component-diagram/AGENTS.md:0-0
Timestamp: 2025-11-25T06:34:10.812Z
Learning: Keep node widgets focused on visual representation; move complex business logic to utility functions and context providers
Applied to files:
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts
🧬 Code graph analysis (2)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/Utils.ts (1)
page(39-39)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/Utils.ts (2)
initTest(179-220)page(39-39)
🪛 Biome (2.1.2)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts
[error] 31-31: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 82-82: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 110-110: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 138-138: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts
[error] 29-29: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 75-75: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 99-99: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
[error] 123-123: Unexpected empty object pattern.
(lint/correctness/noEmptyPattern)
🔇 Additional comments (14)
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/DataMapperUtils.ts (7)
27-34: LGTM!The
DataMapperclass constructor andwaitFormethod are well-structured. The class properly encapsulates the webView Frame and provides a clean initialization pattern.
54-64: Robust clickability check implementation.The
isClickablemethod correctly checks if an element is obstructed by verifying the element at the center point. The logic handles parent-child containment relationships well.
186-191: Good practice: Normalizing line endings for cross-platform compatibility.The
compareFilesSyncfunction correctly normalizes\r\nto\nfor consistent comparisons across Windows and Unix systems.
196-319: Well-structured test scenario with comprehensive mapping coverage.The
testBasicMappingsfunction covers:
- Direct field mappings
- Error handling with diagnostic verification
- Many-to-one mappings
- Object mappings with menu options
- Deletion workflows
- File verification at each step
The console logging provides good visibility into test progress.
321-458: Comprehensive array inner mapping test scenario.The
testArrayInnerMappingsfunction effectively tests:
- Query expression mappings
- Focused view navigation (breadcrumbs and back button)
- Array initialization and element addition
- Element deletion workflows
Good coverage of the array inner mapping requirements from the PR objectives.
460-598: Thorough array root mapping test scenario.The
testArrayRootMappingsfunction covers nested focused views, intermediate query expression deletion, and root array manipulation. The test flow aligns well with the PR objectives for array-to-array workflows.
80-84: Verify that__refreshDMis exposed in the webView context.The
refreshmethod callswindow.__refreshDM()which must be exposed by the Data Mapper component. Ensure this global hook is available to avoid runtime failures.workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/inline-data-mapper.spec.ts (4)
36-37: Test setup properly initializes project files.The test correctly prepares the project state by copying initialization files before each test action. This ensures test isolation and reproducibility.
41-44: Good error handling for missing webView.The null check with a descriptive error message helps identify webView initialization failures quickly.
95-105: Smart conditional navigation based on Data Mapper state.The test correctly checks if Data Mapper is already open (from a previous test or retry) and skips navigation accordingly. This makes tests more resilient to different starting states.
26-165: Well-organized test suite structure.The inline data mapper test suite is well-structured with:
- Clear test naming (
Create,Basic,Array Inner,Array Root)- Consistent logging for test progress visibility
- Proper use of shared test scenarios via
TestScenariosnamespace- Retry attempt tracking for debugging flaky tests
workspaces/bi/bi-extension/src/test/e2e-playwright-tests/data-mapper/reusable-data-mapper.spec.ts (3)
44-63: Comprehensive form filling for reusable data mapper creation.The test properly fills all required fields:
- Data Mapper Name
- Output type selection
- Input parameter with name and type
The use of
{ force: true }for click operations helps with potential overlay issues in the webView.
88-92: Simpler navigation for reusable data mapper.Unlike the inline spec which navigates through the diagram, reusable data mappers are accessed directly via the 'output' treeitem. This appropriately reflects the different access patterns for reusable vs inline mappers.
24-146: Consistent structure with inline data mapper tests.The reusable data mapper test suite mirrors the inline test structure while appropriately differing in:
- File paths (
data_mappings.balvsautomation.bal)- Navigation patterns (treeitem click vs diagram navigation)
- Creation flow (form-based vs declare variable node)
This consistency makes the test suite maintainable.
KCSAbeywickrama
added
Checks/Run BI UI Tests
1 participant
Purpose
Included Test Scenarios
Inline Data Mapper - Create
Open in Data MapperbuttonReusable Data Mapper - Create
Reusable/Inline Data Mapper - Basic
Reusable/Inline Data Mapper - Array Inner
Reusable/Inline Data Mapper - Array Root
Summary by CodeRabbit
Tests
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.