Plugin E2E: Add components fixture with dataSourcePicker and timeRangePicker#2583
Plugin E2E: Add components fixture with dataSourcePicker and timeRangePicker#2583
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Playwright fixture for creating UI component model instances (starting with DataSourcePicker) without requiring page-specific fixtures or manual PluginTestCtx construction.
Changes:
- Extends
PluginFixturewith a newcomponents: Componentsfixture. - Introduces a
Componentsfactory model exposinggetDataSourcePicker(root?). - Wires the new
componentsfixture into the exportedtestfixture set.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/plugin-e2e/src/types.ts | Adds components: Components to the public fixture type and documents usage. |
| packages/plugin-e2e/src/models/Components.ts | New component factory class that constructs DataSourcePicker instances (optionally scoped by a root locator). |
| packages/plugin-e2e/src/index.ts | Exports Components and registers the new components fixture in test.extend(...). |
| packages/plugin-e2e/src/fixtures/components.ts | Implements the Playwright fixture that instantiates Components with PluginTestCtx. |
sunker
left a comment
There was a problem hiding this comment.
Nice Marcus!
It would be quite nice to instantiate all components under /models/components in the Components class constructor. That way the API would be closer to other plugin-e2e APIs:
test('my test', async ({ components }) => {
await components.dataSourcePicker.set('prom');
});Then to scope the component to another root than page, you'd use the same pattern as Playwright's locator.locator(...) - a within() helper that returns a scoped clone:
test('my test', async ({ components, panelEditPage }) => {
const panel = panelEditPage.getPanelByTitle('My panel').locator;
await components.dataSourcePicker.within(panel).set('prom');
});To avoid repeating the within() boilerplate in every component, we could pull it into a small ScopedComponent base class that each component extends. Something like:
export abstract class ScopedComponent extends GrafanaPage {
constructor(
ctx: PluginTestCtx,
protected readonly root?: Locator
) {
super(ctx);
}
within(root: Locator): this {
const Ctor = this.constructor as new (ctx: PluginTestCtx, root?: Locator) => this;
return new Ctor(this.ctx, root);
}
}WDYT?
Expose a fixture that returns a DataSourcePicker instance without requiring manual PluginTestCtx construction. Useful for tests that interact with a datasource picker on pages not covered by existing page fixtures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Expose a `components` fixture that returns a Components factory object. This groups component factories (currently just `getDataSourcePicker`) in one place and makes it easier to add more in the future without bloating the top-level test fixtures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Verify that components.getDataSourcePicker() sets the data source on a panel edit page, both without a root locator and when scoped to the panel editor content. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Expose components as eagerly constructed properties on the Components class (e.g. components.dataSourcePicker.set(...)) and introduce a ScopedComponent base class providing a Playwright-style within(root) method for scoping, mirroring locator.locator(...). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0b0ca89 to
a932d84
Compare
Migrate TimeRange to ScopedComponent and expose it as components.timeRangePicker, following the same pattern as components.dataSourcePicker. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sunker
left a comment
There was a problem hiding this comment.
Approved, but there's one nit to fix
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use .first() on the openButton assertion to avoid strict mode violations on Grafana versions that render the time picker twice. Remove the within() test for timeRangePicker - the within() pattern is covered by dataSourcePicker.within() tests and there is no natural scope for a time range picker in the test environment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…root Use gotoDashboardPage for a stable time picker location. Resolve the root locator based on Grafana version: NavToolbar.container for >=9.4.0 (when it was introduced), PageToolbar.container for older versions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Expose a version-aware toolbar locator on DashboardPage. Resolves to NavToolbar.container on Grafana >=9.4.0 and falls back to .page-toolbar for older versions. Use it in the timeRangePicker.within() test to remove the version logic from the test itself. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In Grafana 11.1.0+ (scenes-based dashboards) the time range controls live inside the `Dashboard.Controls` container, not `NavToolbar.container` (which is the app-level navigation bar). Scope the toolbar getter to `Dashboard.Controls` for >= 11.1.0, keeping NavToolbar for 9.4.0–11.0.x and the CSS fallback for older versions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- DataSourcePicker.set(): fall back to page-level input search when the scoped root doesn't contain the inputV2 element (Grafana 13 Enterprise alerting renders the picker outside the query-editor row) - AlertRuleEditPage.evaluate() route handler: guard against body.results being undefined/differently shaped in Grafana 13 Enterprise; wrap in try/catch so the route is always fulfilled and waitForResponse never hangs - page.ts: merge featureToggles into the OFREP interceptor flags for Grafana >= 12.1.0. The server's OFREP bulk-evaluation response was overriding bootData values (e.g. tlsEnabled, alertingQueryAndExpressionsStepMode) after app load; both bootData and OFREP now carry the same overrides. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Merge featureToggles into OFREP flags so Grafana 13's server-side bulk-eval response no longer overrides bootData values (fixes tlsEnabled and alertingQueryAndExpressionsStepMode toggle tests) - Null-safe route handler in AlertRuleEditPage.evaluate() to handle undefined results in Grafana 13 Enterprise responses - Patch QueryEditorRows/QueryEditorRow selector boundary from 13.1.0 to 13.0.1 to match when the data-testid attributes were actually introduced Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The patch incorrectly moved the data-testid query editor row selector boundary from 13.1.0 to 13.0.1. Grafana 13.0.1 still uses the old aria-label="Query editor row" format; @grafana/e2e-selectors already has the correct 13.1.0 boundary. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Playwright test results
Troubleshooting404 when clicking on
|
| const { TimeZonePicker, TimePicker, Select } = this.ctx.selectors.components; | ||
| try { | ||
| await this.getByGrafanaSelector(TimePicker.openButton).click(); | ||
| await this.getByGrafanaSelector(TimePicker.openButton, { root: this.root }).click(); |
There was a problem hiding this comment.
Curious why we need this change?
There was a problem hiding this comment.
It is because you can do the within which change the scope of the time picker. It should try to find the time picker within that new root element instead of the page.
Does this make sense?
What this PR does / why we need it:
Adds a
componentsfixture that exposes eagerly-constructed Grafana UI components for use in tests on arbitrary pages — not just the ones covered by page-specific fixtures likePanelEditPageorExplorePage.Two components are exposed:
components.dataSourcePicker— useful for tests that interact with a data source picker on any page (e.g. Grafana's exemplars test)components.timeRangePicker— useful for tests that set a time range on any pageBoth components support a Playwright-style
within(root)method for scoping to a specific DOM container (mirroringlocator.locator(...)). Thewithin()method is provided by a new sharedScopedComponentbase class.Which issue(s) this PR fixes:
Special notes for your reviewer:
ScopedComponentis exported as part of the public API so plugin authors can subclass it for their own page-level components. Additional components can be added toComponentsfollowing the same pattern.📦 Published PR as canary version:
Canary Versions✨ Test out this PR locally via:
npm install website@5.6.0-canary.2583.25382670310.0 npm install @grafana/create-plugin@7.4.0-canary.2583.25382670310.0 npm install @grafana/plugin-e2e@3.8.0-canary.2583.25382670310.0 # or yarn add website@5.6.0-canary.2583.25382670310.0 yarn add @grafana/create-plugin@7.4.0-canary.2583.25382670310.0 yarn add @grafana/plugin-e2e@3.8.0-canary.2583.25382670310.0