Update opa5_docu.md based on testing - #5000
Conversation
🦋 Changeset detectedLatest commit: b6b093b The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![hyperspace-pr-bot[bot]](https://avatars.githubusercontent.com/u/2531208?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR makes substantial improvements to the documentation — correcting API usage patterns, adding valuable new sections on V4 fundamentals, value help, and V2 details. Two issues were flagged: the iTearDownMyApp / iTeardownMyApp capitalisation inconsistency that remains in the Category 01 examples (while the rest of the document uses the lowercase-d form), and the removal of the surrounding CustomPageDefinitions object wrapper in the Category 12 custom selector code blocks, which leaves bare function definitions that are invalid JavaScript without context. Please address both before merging.
Co-authored-by: hyperspace-pr-bot[bot] <209611008+hyperspace-pr-bot[bot]@users.noreply.github.com>
|
heimwege
left a comment
heimwege
left a comment
There was a problem hiding this comment.
To me this document as is should not be merged. It must be aligned with the skill or it will produce different/wrong results.
See comments + e.g. QUnit autostart false is completely missing, Not everywhere RAP is mentioned, the CAP counterpart is provided, ...
Prompt:
check the document update at https://github.com/SAP/open-ux-tools/pull/5000 and compare to the information provided in the related skill file at https://github.com/SAP/open-ux-tools/blob/main/packages/fiori-mcp-server/skills/sap-fiori-opa5-test-development/SKILL.md The document in the PR should contain more or less the same information contined in the skill. Nothing should be missing and nothing should be contradicting.
I suggest to finalize the skill (because there are still some findings from Markus open) and then create the documentation based on the skill (maybe also automated generation like we do for the create readme). Else this will be out of sync sooner or later and that's what will lead to issues especially if the MCP and the skill are being used in one plugin (which is the way we recommend.
To be honest I would just delete this document because I don't see the benefit in having bot but I see the effort. But it's not up to me to decide 🤷🏻
| "@sap-ux/fiori-mcp-server": patch | ||
| --- | ||
|
|
||
| FIX: Update opa5_docu.md based on testing |
There was a problem hiding this comment.
I think what this PR should actually do is:
| FIX: Update opa5_docu.md based on testing | |
| FIX: Align opa5_docu.md with sap-fiori-opa5-test-development content |
And then I'm wondering how this can be done because there are currently still some open findings to be integrated into the skill 🤷🏻
| // ❌ Wrong — teardown chained on page object, no prior assertion | ||
| opaTest("Clean up", function(Given, When, Then) { | ||
| Then.onTheList.iSeeThisPage() | ||
| .and.onTheList.iTeardownMyApp(); |
There was a problem hiding this comment.
iTeardownMyApp() is used throughout (lowercase d). Only one line mentions the actual difference in v2 vs v4
(V4: `Given.iTeardownMyApp()`, V2: `Given.iTeardownMyApp()`)
If this snipped is not returned by the search the MCP will lead to wrong code.
|
|
||
| ### Page Object Configuration | ||
|
|
||
| - Register all page objects in `JourneyRunner.pages` before running journeys |
There was a problem hiding this comment.
In contrast to the skill, the JourneyRunner is not explained at all in this documentation
| **Searching within value help:** | ||
|
|
||
| Search via the generic search field. The search field is only available when the value help entity set is annotated as searchable in `metadata.xml`. Depending on the backend: | ||
| - **CAP**: `Search.SearchRestrictions` with `Searchable: true` (from `@Search.searchable: true` in CDS) |
There was a problem hiding this comment.
this is wrong (fixed in the skill)
| - `Searchable: true` present → **Case A** | ||
| - Absent or `Searchable: false` → **Case B** | ||
|
|
||
| > For RAP backends, Case A is the default — standard VH entities expose `SAP__capabilities.SearchRestrictions` with `Searchable: true`. When in doubt, check the metadata rather than assuming Case B. |
3 participants
Description
This PR updates the OPA5 integration test documentation (
opa5_docu.md) based on hands-on testing feedback. The changes correct inaccuracies, add missing patterns, and expand both the V4 and V2 API reference sections to better reflect real-world usage.Key improvements include:
TemplatePageand FCL notes, replaced the teardown rule with a correct code example showingGiven.iTeardownMyApp()called separately (not chained), and added a new Debugging section with thequnitPausetip..and.vs.and.then.chaining rules.{ property: "..." },{ section: "..." }). ClarifiediCheckFieldbehavior for text-annotated fields ({ value, description }), section ID vs. display label distinction,iChangeSortOrderAPI, and value help selection patterns (single-select vs. multi-select, Case A vs. Case B).onTheShell.iNavigateBack()andThen.iSeeMessageToast().onTheGenericListReport,onTheGenericObjectPage, ALP, FCL), a setup/configuration guide, app startup snippet, common pitfalls, and a complete working example journey.Do NOT chain teardown on a page object, fixed method name casingiTeardownMyApp).Type of change
How have you tested?
Changes are documentation-only. Patterns were verified through actual test execution against SAP Fiori Elements applications.
Checklist:
The code conforms to the general development principles
Supplied as many details as possible on this change
The code is easy to read and maintainable by others
Corresponding changes to the documentation has been done
Already existing and new unit tests pass locally
🔄 Regenerate and Update Summary
PR Bot Information
Version:
1.29.9anthropic--claude-4.6-sonnet44d5c490-8bf3-11f1-894b-56c5da534b3cpull_request.opened