Skip to content

feat(plugin-e2e): expose response body from waitForQueryDataResponse - #2854

Open
adamyeats wants to merge 2 commits into
mainfrom
adamyeats/expose-query-response-body
Open

feat(plugin-e2e): expose response body from waitForQueryDataResponse#2854
adamyeats wants to merge 2 commits into
mainfrom
adamyeats/expose-query-response-body

Conversation

@adamyeats

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

GrafanaPage.waitForQueryDataResponse only ever hands the caller a raw Playwright Response — its cb predicate is (request: Response) => boolean | Promise<boolean>, with no way to filter or assert on the response body without a separate .json() call. Reading the body after the response has resolved is unreliable in practice (the CDP buffer isn't guaranteed to still be live), so plugin authors end up re-implementing "parse the body inside the predicate, stash it in a closure" by hand — that's exactly what grafana/clickhouse-datasource did, and where this suggestion came from (review feedback on clickhouse-datasource#2122, no existing upstream issue to link).

This PR adds a new, purely additive waitForQueryDataResponseWithBody method alongside the existing waitForQueryDataResponse, on the shared GrafanaPage base class so every page model (ExplorePage, DashboardPage, PanelEditPage, AlertRuleEditPage) gets it for free. It returns { response, body }, and its predicate receives the parsed body so callers can filter by response contents (e.g. a specific refId's frames) rather than only status/url. Tagged @alpha following the precedent of DashboardPage.waitForPanelsQueriesToComplete, since the exact return shape may want to evolve.

The existing waitForQueryDataResponse is untouched — nothing about this is a breaking change.

Which issue(s) this PR fixes:

None filed — this came directly out of review feedback on an external plugin's PR (see above). Happy to file one first if preferred.

Special notes for your reviewer:

  • Verified locally against grafana-enterprise:11.4.0 via npm run server + npx playwright test: the two new tests pass, and the full existing queryEditor.integration.spec.ts/queryEditor.spec.ts suites pass unchanged.
  • Ran the full package test suite (240 tests) for regressions: 3 failures under default parallelism turned out to be pre-existing flakiness unrelated to this change — I confirmed by re-running the same specs with this change reverted (a different, larger set of 6 failed) and by re-running with --workers=1 (all passed, both with and without this change). No test that exercises GrafanaPage/waitForQueryDataResponse regressed.
  • Naming: went with waitForQueryDataResponseWithBody to mirror clickhouse-datasource's own local helper, for continuity with the code this is generalising from. Open to a different name if you'd prefer something else.

@sunker
sunker self-requested a review August 27, 2026 08:18
@grafana-catalog-project-bot grafana-catalog-project-bot Bot moved this from 📬 Triage to 🧑‍💻 In development in Grafana Catalog Team Aug 27, 2026
sunker
sunker previously approved these changes Aug 27, 2026

@sunker sunker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice 🚀 If you have a good, generic use case for this, you can add an example somewhere in the ds docs.

@github-project-automation github-project-automation Bot moved this from 🧑‍💻 In development to 🔬 In review in Grafana Catalog Team Aug 27, 2026
@adamyeats

Copy link
Copy Markdown
Contributor Author

@sunker Good call! Added some docs for this 👍

@adamyeats
adamyeats marked this pull request as ready for review August 27, 2026 12:50
@adamyeats
adamyeats requested review from a team as code owners August 27, 2026 12:50
@adamyeats
adamyeats requested review from Ukochka, ashharrison90, jackw, toddtreece and xnyo and removed request for a team August 27, 2026 12:50
@adamyeats
adamyeats force-pushed the adamyeats/expose-query-response-body branch from 750dc90 to 977ed36 Compare August 27, 2026 12:51

@sunker sunker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🔬 In review

Development

Successfully merging this pull request may close these issues.

2 participants