test(exapp_integration): add a browser test that the admin settings page renders - #1033
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughAdds a Playwright test for the AppAPI admin settings page. The test records uncaught page errors and HTTP error responses, then verifies the AppAPI and Deploy daemons headings, the configured daemon name, and the Register daemon button. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to The test change is otherwise ready, but the documented Playwright server-version behavior remains inaccurate for stable-branch pull requests. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 1
🧹 Nitpick comments (1)
.github/workflows/playwright.yml (1)
85-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRun the Playwright npm script in CI.
This command bypasses
npm run playwright, which is the documented local entry point. If the script gains required options or setup, local and CI execution will diverge.Proposed fix
- run: npx playwright test + run: npm run playwrightBased on learnings, use the composer/npm scripts because these are what CI runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 73dcde79-dfb0-4655-aef1-73d403d35f29
⛔ Files ignored due to path filters (4)
js/app_api-adminSettings.js.mapis excluded by!**/*.mapjs/app_api-vendors-node_modules_nextcloud_dialogs_dist_chunks_FilePicker-W-IYpVkn_mjs.js.mapis excluded by!**/*.mapjs/app_api-vendors-node_modules_nextcloud_dialogs_dist_chunks__plugin-vue_export-helper-1tPrXgE0_mjs-nod-dc4ad5.js.mapis excluded by!**/*.mappackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.github/workflows/playwright.yml.gitignoreAGENTS.mdjs/app_api-adminSettings.js.licensejs/app_api-node_modules_nextcloud_dialogs_dist_chunks_PublicAuthPrompt-uFWxS9Cy_mjs.js.licensejs/app_api-vendors-node_modules_mdi_js_mdi_js-node_modules_nextcloud_dialogs_dist_chunks_preview-BIbJGxX-e6bda3.js.licensejs/app_api-vendors-node_modules_nextcloud_dialogs_dist_chunks_ConflictPicker-D-B-WV0C_mjs.js.licensejs/app_api-vendors-node_modules_nextcloud_dialogs_dist_chunks_FilePicker-W-IYpVkn_mjs.js.licensejs/app_api-vendors-node_modules_nextcloud_dialogs_dist_chunks__plugin-vue_export-helper-1tPrXgE0_mjs-nod-dc4ad5.js.licensepackage.jsonplaywright.config.tstests/playwright/e2e/admin-settings.spec.tstests/playwright/start-server.mjs
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.
| - `phpunit.yml`: `composer test:unit` on PHP 8.3 and 8.4. | ||
| - `js-test.yml`: vitest on `src/**` changes. | ||
| - `playwright.yml`: builds the frontend and runs the Playwright E2E tests in `tests/playwright/` against a | ||
| Nextcloud test container (server `master`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe the server branch selection accurately.
The workflow passes github.base_ref for pull requests. The server script maps only main to master. A pull request targeting stable32 therefore tests stable32, not master.
Proposed fix
- Nextcloud test container (server `master`).
+ Nextcloud test container (the pull request base branch; `main` maps to server `master`).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Nextcloud test container (server `master`). | |
| Nextcloud test container (the pull request base branch; `main` maps to server `master`). |
…age renders Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
a32c6a0 to
ec3ea5f
Compare
oleksandr-nc
changed the title
|
/backport to stable35 |
|
/backport to stable34 |
1 participant
On stable32 the AppAPI admin settings page has rendered empty since 32.0.13 (fixed in #1034). Dependabot PR #915 deduped
@nextcloud/l10nto 3.4.1 under@nextcloud/vue8.17.1, whose bundled translation code writes to a gettext internal that@nextcloud/l10n3.2 removed, so the script throwsTypeError: Cannot read properties of undefined (reading 'catalogs')while loading. Nothing in CI opened the page, so the bump and its recompiled assets shipped.This adds
test_admin_settings.pyto the ExApp integration suite, next to the FileActionMenu test for #848: it opens the admin settings page as admin and asserts that there are no uncaught errors or failingapp_apirequests, and that the headings, the daemon list withmanual_daemonand the register button render. The suite's job already builds the frontend from the PR's lockfile, so a dependency bump like #915 now fails before it merges.Verified locally: the test passes against server master and fails with the
catalogserror when the stable32 admin bundle is swapped in.