Skip to content

Comments

Fix E2E tests#979

Merged
gigara merged 2 commits intowso2:mainfrom
ChinthakaJ98:e2e-test-fixes-02
Nov 20, 2025
Merged

Fix E2E tests#979
gigara merged 2 commits intowso2:mainfrom
ChinthakaJ98:e2e-test-fixes-02

Conversation

@ChinthakaJ98
Copy link
Contributor

@ChinthakaJ98 ChinthakaJ98 commented Nov 20, 2025

$subject

Summary by CodeRabbit

  • Tests
    • Disabled a Ballerina module test (temporarily skipped).
    • Added and reworked end-to-end coverage for custom function mapping flows in the data mapper.
    • Improved editor focus handling during mapping edits to stabilize test interactions.
    • Updated test mapping examples to include an uppercase transformation and expanded multi-input mapping cases.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 20, 2025

Walkthrough

Disabled one e2e Ballerina module test and updated data-mapper test fixtures and e2e specs: adjusted mapping transformations (added oExp, expanded oManyOne/oManyOneErr, reordered properties) and added/modified custom-function mapping test flows with focus/blur management.

Changes

Cohort / File(s) Summary
Test Disabling
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/artifactTests/artifact.spec.ts
Replaced an active Ballerina Module test with a skipped test (test.skip) and added a comment referencing re-enablement after an issue is resolved.
Data Mapper Test Fixtures
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/del.ts, workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/map.ts
Updated mapping logic in del.ts: oManyOne now computed from three inputs, oManyOneErr expanded to include iManyOne3, added oExp using dmUtils.toUppercase, and minor trailing-comma/ordering edits. In map.ts reordered oCustomFn and oExp properties (no functional change).
Data Mapper E2E Tests
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/dataMapper.spec.ts
Added new test steps for custom-function mapping insertion and verification (connectivity assertions, editor tab open/close/detach), removed a duplicate block, and added explicit blur actions to manage expression-bar focus.

Sequence Diagram(s)

sequenceDiagram
    participant Test as E2E Test
    participant UI as DataMapper UI
    participant DM as DataMapper Engine
    participant Editor as Mapping Editor
    participant Node as Connector Node

    rect rgb(240,248,255)
    Test->>UI: insert custom-function block (menu-item-o2o-func)
    UI->>DM: create mapping link between input and custom function
    DM-->>UI: mapping link established
    end

    rect rgb(245,255,240)
    UI->>Editor: open mapping editor tab
    Editor->>Editor: edit expression (set iCustomFn -> oCustomFn)
    Editor->>UI: blur expression bar (focus out)
    Editor-->>UI: close/detach editor tab
    end

    rect rgb(255,250,240)
    UI->>Node: verify connector nodes linked (input, intermediate, output)
    Node-->>Test: connectivity verified
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect del.ts arithmetic/concatenation logic for correctness and test expectations.
  • Confirm map.ts property reordering has no behavioral impact on tests.
  • Validate new assertions and focus/blur steps in dataMapper.spec.ts for flakiness risks and timing/wait logic.
  • Verify the skipped test in artifact.spec.ts includes clear TODO/comment and references.

Suggested reviewers

  • hevayo
  • gigara

Poem

🐰 A skip, a map, a tiny change in view,
Three inputs join, a new uppercase too.
I hop, I test, I blur the bar with grace,
Mappings click, editor leaves its place.
Hooray — the rabbit smiles at every trace.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description is entirely missing; only '$subject' placeholder text is present, failing to provide any Purpose, Goals, Approach, or other required template sections. Complete the pull request description using the provided template, including Purpose (with issue links), Goals, Approach, and other relevant sections for this test fix.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fix E2E tests' is vague and generic, using non-descriptive terminology that doesn't convey what specific E2E tests are being fixed or what the actual changes accomplish. Provide a more specific title that describes the particular tests being fixed, such as 'Fix custom function mapping and property ordering in data mapper E2E tests' or similar.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between da64194 and dd72d4c.

📒 Files selected for processing (4)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/artifactTests/artifact.spec.ts (1 hunks)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/del.ts (1 hunks)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/map.ts (1 hunks)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/dataMapper.spec.ts (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/dataMapper.spec.ts
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/del.ts
🧰 Additional context used
🧠 Learnings (1)
📚 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/mi/mi-extension/src/test/e2e-playwright-tests/artifactTests/artifact.spec.ts
🔇 Additional comments (2)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/map.ts (1)

76-78: Non-functional change; mapping remains consistent

The updated return object keeps the same mappings for oCustomFn and oExp; only ordering/formatting changed. No issues from a behavior or test-fixture perspective.

workspaces/mi/mi-extension/src/test/e2e-playwright-tests/artifactTests/artifact.spec.ts (1)

259-279: Good use of test.skip with clear re-enable condition

Switching the Ballerina Module test to test.skip (with a comment pointing to the upstream issue) is a clean way to keep the scenario runnable and visible while unblocking the suite. Just remember to drop the skip when ballerina-lang#44401 is resolved.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/dataMapper.spec.ts (1)

193-207: Custom function coverage and focus handling look consistent with golden files

The new custom-function block (input.iCustomFn → objectOutput.oCustomFn via menu-item-o2o-func), the tab open/close checks, and the explicit expressionBar blur all line up with the updated basic/map.ts and basic/del.ts expectations. Commenting out the extra delete checks with a TODO and issue link keeps the suite stable while clearly marking pending work. No issues spotted in these additions.

If you notice flakiness around the editor-tab locator, consider relaxing the getByRole('tab', { name: ... }) match to rely primarily on the file name (e.g., regex on ${dmName}.ts) so minor VS Code label changes don’t break the test. Please rerun the Data Mapper E2E group after these updates to confirm stability.

Also applies to: 238-239, 260-272

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c70be34 and da64194.

📒 Files selected for processing (4)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/artifactTests/artifact.spec.ts (1 hunks)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/del.ts (1 hunks)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/map.ts (1 hunks)
  • workspaces/mi/mi-extension/src/test/e2e-playwright-tests/dataMapper.spec.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/dataMapper.spec.ts (1)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/Utils.ts (1)
  • page (39-39)
🔇 Additional comments (3)
workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/del.ts (1)

64-76: del.ts golden output now matches updated Basic Mappings flow

Including iManyOne3 in both oManyOne/oManyOneErr and adding oCustomFn/oExp assignments aligns this golden file with the new mappings exercised in testBasicMappings, while still representing the post-delete state (prim mappings removed). Looks consistent and intentional for the E2E expectations.

workspaces/mi/mi-extension/src/test/e2e-playwright-tests/artifactTests/artifact.spec.ts (1)

259-279: Ballerina Module tests cleanly disabled with upstream issue reference

Commenting out the Ballerina Module Tests block and adding a pointer to ballerina-lang issue 44401 makes the temporary disablement explicit without affecting other artifact tests. This is reasonable for stabilizing the suite until the upstream issue is fixed.

workspaces/mi/mi-extension/src/test/e2e-playwright-tests/data/datamapper-files/basic/map.ts (1)

64-78: map.ts property ordering aligned with new mappings

Reordering the return object so oCustomFn precedes oExp without changing the expressions themselves keeps this golden file in sync with the updated Data Mapper behavior and the testBasicMappings expectations.

@ChinthakaJ98 ChinthakaJ98 removed the Checks/Run MI UI Tests Force run MI UI tests label Nov 20, 2025
@gigara
Copy link
Contributor

gigara commented Nov 20, 2025

@gigara gigara merged commit ebeb9be into wso2:main Nov 20, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants