-
Notifications
You must be signed in to change notification settings - Fork 3
feat(ui5): Add OPA5 guidelines as Skills #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
057ffc2
feat(ui5): Add OPA5 guidelines as Skills
kineticjs 934a92b
feat(ui5): Correct formatting
kineticjs 6931ec1
feat(ui5): Reflect review feedback on skill content
kineticjs daea194
feat(ui5): Add readme entry for opa5 skill
kineticjs f3666fe
feat(ui5): Add keyword entry for opa5 skill in plugin.json
kineticjs 6cc7ee8
feat(ui5): Split skill based on phase applicability
kineticjs 1d9bf50
feat(ui5): Minor improvements in formating and text
kineticjs e59d241
feat(ui5): Rename skill to existing convention
kineticjs b89dadd
feat(ui5): Improve text
kineticjs ed3d277
feat(ui5): Improve title
kineticjs 7efc160
feat(ui5): Improve title
kineticjs 1537e99
feat(ui5): Adapt TestRecorder example to use async/await
kineticjs 94e1bf6
feat(ui5): Adapt to new object syntax
kineticjs e2ca5fd
feat(ui5): Adapt to new object syntax
kineticjs fdacddd
feat(ui5): Align OPA5 skill headings to Title Case
kineticjs 5a1fcc6
feat(ui5): Rename skill folder to match the name
kineticjs 9203b82
feat(ui5): Correct instruction scope
kineticjs 4b713de
feat(ui5): Code snippet improvements
kineticjs 4c7d3e0
feat(ui5): Adapt to modern syntax
kineticjs b984a94
feat(ui5): Adapt to modern syntax
kineticjs 59a269b
feat(ui5): Add missing use strict
kineticjs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| "ui5", | ||
| "sapui5", | ||
| "openui5", | ||
| "opa5", | ||
| "plugin", | ||
| "linter", | ||
| "api-documentation", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| "ui5", | ||
| "sapui5", | ||
| "openui5", | ||
| "opa5", | ||
| "plugin", | ||
| "linter", | ||
| "api-documentation", | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| --- | ||
| name: ui5-best-practices-opa5 | ||
| description: This skill should be used in any OPA5 task - creating, modifying, extending, debugging, fixing or reviewing an integration test. Use when the user asks to "write an OPA5 test", "add an OPA5 journey", "fix the OPA5 test failure" or mentions OPA5 or its components - opaTest, page object, journey, waitFor. | ||
| --- | ||
|
|
||
| # OPA5 Guidelines and Tools | ||
|
|
||
| ## Handle Special Cases (follow when planning and writing an OPA5 test) | ||
| - **Initial Configuration for OPA5 Test** → follow `references/configuration.md` | ||
| - **If the test-case spans multiple views** → follow `references/handle-multiple-views.md` | ||
| - **Teardown the App** → follow `references/handle-teardown.md` | ||
|
|
||
| ## Set Up Browser Inspection Tools (follow **before running the OPA5 test**) | ||
| **Purpose:** Efficient inspection of test failures with minimal steps. | ||
| **Prerequisites:** A tool to load the OPA5 test in the browser and evaluate javascript in the browser window (e.g. MCP Playwright) | ||
| **Instructions:** → follow `references/setup-inspection-tools.md` |
26 changes: 26 additions & 0 deletions
26
plugins/ui5/skills/ui5-best-practices-opa5/references/configuration.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| # Configuration | ||
|
|
||
| 1. Use this folder layout: | ||
| ``` | ||
| test/integration/ | ||
| ├── opaTests.qunit.js ← single entry point | ||
| ├── pages/ | ||
| │ ├── Welcome.js ← one page object per view (name matches the view) | ||
| │ ├── Items.js | ||
| │ └── Browser.js ← cross-view actions (navigation, hash) | ||
| ├── WelcomeJourney.js ← one journey per feature/functionality | ||
| └── FilterItemsJourney.js | ||
| ``` | ||
|
|
||
| 2. **ALWAYS** enable `autoWait` and define `viewNamespace` globally in `opaTests.qunit.js`. | ||
| ```javascript | ||
| // opaTests.qunit.js | ||
| sap.ui.define(["sap/ui/test/Opa5"], (Opa5) => { | ||
| "use strict"; | ||
| Opa5.extendConfig({ | ||
| autoWait: true, | ||
| viewNamespace: "com.myorg.myapp.view." | ||
| }); | ||
| // ... | ||
| }); | ||
| ``` | ||
65 changes: 65 additions & 0 deletions
65
...ns/ui5/skills/ui5-best-practices-opa5/references/enable-testrecorder-tooling.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| # TestRecorder Tooling | ||
|
|
||
| The `sap.ui.testrecorder` library provides the module `sap.ui.testrecorder.ControlTree` that allows to: | ||
| - inspect the live control tree in the browser | ||
| - retrieve reliable OPA5 snippets for interacting with any part of the control tree | ||
|
|
||
| ## Prerequisites to Use `sap.ui.testrecorder.ControlTree` | ||
|
|
||
| - **UI5 version ≥ 1.147** | ||
| - Tool to load the OPA5 test in the browser and evaluate javascript in the browser window (e.g. MCP Playwright) | ||
| - **`sap.ui.testrecorder` library loaded** — temporarily add to the app's library declarations in the places listed below (**ORDERED BY PRIORITY**): | ||
| 1. `ui5.yaml` → `framework.libraries`: `- name: sap.ui.testrecorder` | ||
| 2. `manifest.json` → `sap.ui5.dependencies.libs`: `"sap.ui.testrecorder": {}` | ||
| 3. `index.html` → `data-sap-ui-libs` bootstrap attribute: append `,sap.ui.testrecorder` | ||
|
|
||
| > After adding to `ui5.yaml` ensure the server is serving the added library before proceeding: | ||
| > ```bash | ||
| > curl -s -o /dev/null -w "%{http_code}" \ | ||
| > http://localhost:8080/resources/sap/ui/testrecorder/ControlTree.js | ||
| > ``` | ||
| > If 404, **ALWAYS** start a fresh server on the next free port (8081, 8082, …) and use that port | ||
| > for all subsequent browser navigation | ||
|
|
||
| > Remove `sap.ui.testrecorder` after use — not needed at runtime. | ||
| > Kill after use any started fresh server instance. | ||
|
|
||
| ## `sap.ui.testrecorder.ControlTree` API | ||
|
|
||
| **`ControlTree.search(query)`** — Search the live UI5 control tree. | ||
| - Returns `Promise<string>` — a tree snapshot with matching controls and their parents | ||
| - `query=""` returns the full tree; `query="anchorBar"` returns filtered results | ||
| - Matches against control type short names, non-default property values, and accessibility attributes | ||
| - Each node carries a `nodeId="N_M"` (snapshot N, node M) — use these in `ControlTree` methods that require a `nodeId` parameter | ||
|
|
||
| **`ControlTree.getControlData(nodeId)`** — Get selector and full control state. | ||
| - Returns `Promise<{ selectorSnippet, properties, aggregations, associations, bindings }>` | ||
| - `selectorSnippet` — OPA5 `waitFor` code to locate the control (use as the base selector) | ||
| - Other fields provide live control state for customizing assertions | ||
|
|
||
| **`ControlTree.press(nodeId, settings?)`** — Press a control and get its OPA5 action snippet. | ||
| - Returns `Promise<string>` — an OPA5 `waitFor` snippet with `actions: new Press()` | ||
| - Also **replays the press** on the running app, advancing the UI state for the next search | ||
| - Optional `settings`: `altKey`, `ctrlKey`, `shiftKey`, `xPercentage`, `yPercentage` | ||
|
|
||
| **`ControlTree.enterText(nodeId, settings)`** — Type into a control and get its OPA5 action snippet. | ||
| - Returns `Promise<string>` — an OPA5 `waitFor` snippet with `actions: new EnterText()` | ||
| - Also **replays the text entry** on the running app | ||
| - `settings`: `text`, `clearTextFirst` (default `true`), `submitText` (default `true`) | ||
|
|
||
| ## Example Usage | ||
|
|
||
| ```javascript | ||
|
kineticjs marked this conversation as resolved.
|
||
| sap.ui.require(["sap/ui/testrecorder/ControlTree"], async (ControlTree) => { | ||
| "use strict"; | ||
| // Navigate to the state where the anchor bar is visible, then: | ||
|
kineticjs marked this conversation as resolved.
|
||
| await ControlTree.search("anchorBar"); // When resolved, inspect the returned markdown snapshot and pick nodeId, e.g. Button nodeId="1_8" text="Methods" | ||
|
|
||
| // Use the picked nodeId to interact with the corresponding control | ||
| await ControlTree.press("1_8"); // When resolved, save the returned OPA5 snippet; UI has now navigated | ||
|
|
||
| await ControlTree.search("selectedSection"); // When resolved, parse returned snapshot and pick nodeId, e.g. ObjectPageLayout nodeId="2_3" | ||
|
|
||
| await ControlTree.getControlData("2_3"); // When resolved, save result.selectorSnippet + result.associations → build assertion | ||
| }); | ||
| ``` | ||
35 changes: 35 additions & 0 deletions
35
plugins/ui5/skills/ui5-best-practices-opa5/references/handle-multiple-views.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # Page Object Organization Across Multiple Views | ||
|
|
||
| **ALWAYS** add the new actions/assertions to the semantically corresponding page object | ||
|
|
||
| Example 1: | ||
| ❌ Anti-Pattern: | ||
| Adding selector for a control from `App.view.xml` into the page object for its **nested** view (e.g. into `integration/pages/Detail.js` for `Detail.view.xml`): | ||
| ```javascript | ||
| // integration/pages/Detail.js | ||
| iShouldSeeTheAppInFullScreenMode() { | ||
| return this.waitFor({ | ||
| id: "layout", | ||
| viewName: "App", | ||
| success: function () { ... } | ||
| }); | ||
| }, | ||
| ``` | ||
| ✅ Correct Pattern: | ||
| Place the assertion for the `App.view.xml` in page object file `integration/pages/App.js` | ||
|
|
||
| Example 2: | ||
|
flovogt marked this conversation as resolved.
|
||
| ❌ Anti-Pattern: | ||
| View-specific page object file containing selector for cross-view navigation: | ||
| ```javascript | ||
| // integration/pages/Detail.js | ||
| iShouldSeeTheHash(sExpectedHash) { | ||
| return this.waitFor({ | ||
| success: function () { | ||
| Opa5.assert.strictEqual(Opa5.getHashChanger().getHash(), sExpectedHash, "The Hash not correct"); | ||
| } | ||
| }); | ||
| }, | ||
| ``` | ||
| ✅ Correct Pattern: | ||
| Place the actions/assertions for cross-view navigation into page object `integration/pages/Browser.js` | ||
26 changes: 26 additions & 0 deletions
26
plugins/ui5/skills/ui5-best-practices-opa5/references/handle-teardown.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| # Teardown the App | ||
|
|
||
| QUnit requires assertions to validate tests. Teardown methods are NOT assertions. | ||
|
|
||
| ❌ Incorrect: | ||
| ```javascript | ||
| opaTest("Should clean up", function(Given, When, Then) { | ||
| Then.iTeardownMyApp(); // ❌ missing assertion (because teardown is not an assertion) | ||
| }); | ||
| ``` | ||
|
|
||
| ❌ Incorrect: | ||
| ```javascript | ||
| opaTest("Should assert state and clean up", function(Given, When, Then) { | ||
| Then.onTheWorklistPage.iShouldSeeTheTable() | ||
| .and.onTheWorklistPage.iTeardownMyApp(); // ❌ chaining on wrong object | ||
| }); | ||
| ``` | ||
|
|
||
| ✅ Correct: | ||
| ```javascript | ||
| opaTest("Should assert state and clean up", function(Given, When, Then) { | ||
| Then.onTheWorklistPage.iShouldSeeTheTable() // ✅ assertion before teardown | ||
| .and.iTeardownMyApp(); // ✅ correct chaining | ||
| }); | ||
| ``` |
24 changes: 24 additions & 0 deletions
24
plugins/ui5/skills/ui5-best-practices-opa5/references/setup-inspection-tools.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Set Up Browser Inspection Tools | ||
|
|
||
| **Prerequisites:** A tool to load the OPA5 test in the browser and evaluate javascript in the browser window (e.g. MCP Playwright) | ||
|
|
||
| ## 1. Set Up TestRecorder Tooling (UI5 version ≥ 1.147 only) | ||
| **Purpose:** | ||
| - Diagnose issues by inspecting the live control tree in the browser, including private/internal controls the test needs to find; | ||
| - Collect reliable OPA5 snippets for non-trivial actions and assertions. | ||
| **Setup:** Follow `enable-testrecorder-tooling.md` for detailed instructions. | ||
|
|
||
| ## 2. Enable Pause-on-Failure Mode (all UI5 versions) | ||
| **Purpose:** When enabled, execution pauses on the first test failure and the app remains live in the browser exactly as it was at the point of failure — no teardown, no reload happens automatically. The paused state persists until you explicitly navigate away, so you can inspect the actual UI directly (without reloading) in the browser to see why it differs from what the test expected. | ||
| **Setup:** Add the following line to your test entry point (right before `Opa5.extendConfig`): | ||
| ```javascript | ||
| // Inside the existing sap.ui.define callback in your test entry point | ||
| sap.ui.test.qunitPause.pauseRule = "assert,timeout"; // enables pause on assertion failures and timeouts | ||
| // Opa5.extendConfig({...}); | ||
| ``` | ||
|
|
||
| ## Workflow | ||
| 1. Enable the inspection tools above and load the test in the browser. | ||
| 2. When the test pauses on failure, inspect the app in the browser. Before changing any code, verify the full causal chain with no gaps. Rule out app-side issues before assuming the test is wrong. | ||
| 3. Iterate on the test until all journeys pass. | ||
| 4. Once all journeys pass, remove the `sap.ui.testrecorder` library from the app and the pause-on-failure rule `sap.ui.test.qunitPause.pauseRule`. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.