Plugin E2E: Make it possible to test query editor in panel edit page #551

sunker · 2023-11-21T07:39:12Z

What this PR does / why we need it:
This PR adds fixtures, models and expect matchers that enable testing a data source query editor in the panel edit page.

A few things to note:

Added two playwright tests files that tests the google sheets data source. One of them are doing full E2E tests, i.e requests are hitting downstream APIs. The other file is only testing the frontend by mocking the Grafana backend.
I've made the GH action matrix multi-dimensional so that tests are not only targeting multiple versions of Grafana, but also multiple versions of the Google Sheets data source.

Sorry for the large amount of files changed. Wanted the PR to include all the pieces that is needed to test a query editor so one can review this as a full feature. Let me know if you want me to try and split the PR.

Which issue(s) this PR fixes:
Part of grafana/grafana#78078

Fixes #

Special notes for your reviewer:

github-actions · 2023-11-21T07:39:31Z

Hello! 👋 This repository uses Auto for releasing packages using PR labels.

✨ This PR can be merged. It will not be considered when calculating future versions of the npm packages and will not appear in the changelogs.

sunker · 2023-11-21T08:49:34Z

packages/plugin-e2e/tsconfig.json

@@ -1,7 +1,8 @@
 {
 "compilerOptions": {
 "outDir": "./dist",
- "declaration": true
+ "declaration": true,
+ "stripInternal": true


Would like some types to be internal so added this option. Maybe this should go in the base config? Thoughts @jackw?

Just out of curiosity - how do we decide which types should be private and which ones should be public? I sometimes find it useful during development to have access to as many types as possible when working with a package, while on the other hand it can tie our hands if we expose too much, as we can hardly change what a lot of people depend on.

Good question. Think I just don't want to overload consumer with irrelevant types etc. But may be better to save this for later.

leventebalogh

Nice work, I really like the explanatory comments, they helped a lot with reviewing. 👏 Generally looks good to me, left a few comments.

packages/plugin-e2e/src/api.ts

leventebalogh · 2023-11-24T13:50:33Z

packages/plugin-e2e/src/api.ts

+ *
+ * If you have tests that depend on the the existance of a data source,
+ * you may use this command in a setup project. Read more about setup projects
+ * here: https://playwright.dev/docs/auth#basic-shared-account-in-all-tests


Just an idea: I think this comment is ok as it is and already really useful, however maybe in the future we could add some Grafana-related examples which we could link to from here as well.

Yes 100%! Actually this comment was made before I knew that it's possible to use env variables in provisioning files. That kind of makes this comment obsolete. I'll change it. But we should definitely have examples of using provisioning with (and without) secrets in plugin-examples eventually.

leventebalogh · 2023-11-24T14:01:15Z

packages/plugin-e2e/src/models/TimeRange.ts

+ await this.ctx.page
+ .getByLabel(this.ctx.selectors.components.PanelEditor.General.content)
+ .locator(`selector=${this.ctx.selectors.components.TimePicker.openButton}`)
+ .click({ force: true, timeout: 2000 });


Why do we need the 2000ms timeout here?

Good question...this is mostly copied from plugin/e2e in grafana core. I'll remove it and we'll see. If timeout is not provided explicitly like in this case, timeout from playwright config will be used. It defaults to 5000ms if I recall correctly. Will changed this!

leventebalogh · 2023-11-24T14:01:56Z

packages/plugin-e2e/src/models/TimeRange.ts

+ .click({ force: true, timeout: 2000 });
+ } catch (e) {
+ // seems like in older versions of Grafana the time picker markup is rendered twice
+ await this.ctx.page.locator('[aria-controls="TimePickerContent"]').last().click();


Could it be that the error is caused by something else?

Unlikely, but maybe. Anyway, I don't like this so I'll add a todo for finding a better solution for this. Have a bunch of todos throughout the code...most of them involves changes in Grafana ui so would like to collect a few before I get to them.

leventebalogh · 2023-11-24T14:05:42Z

packages/plugin-e2e/tests/datasource/configEditor.spec.ts

@@ -11,6 +11,6 @@ test('valid credentials should return a 200 status code', async ({ createDataSou
 const configPage = await createDataSourceConfigPage({ type: 'grafana-googlesheets-datasource' });
 await page.getByText('Google JWT File', { exact: true }).click();
 await page.getByTestId('Paste JWT button').click();
- await page.getByTestId('Configuration text area').fill(process.env.GOOGLE_JWT_FILE!);
+ await page.getByTestId('Configuration text area').fill(process.env.GOOGLE_JWT_FILE!.replace(/'/g, ''));


Just out of curiosity, why do we need to remove the ' single quotes here?

This variable is also used here in the provisioning file for google sheets. Seems like the jwt file needs to be wrapped in single quotes for provisioning to work, so that's how the GH secret is defined. That however doesn't work when creating a new sheets ds using the UI, so therefore I'm removing the single quotes.

packages/plugin-e2e/tests/datasource/queryEditor.integration.spec.ts

leventebalogh · 2023-11-24T14:12:41Z

packages/plugin-e2e/tests/datasource/queryEditor.spec.ts

+
+ await panelEditPage.refreshPanel();
+ await expect(await queryReq).toBeTruthy();
+});


What's the "semantic" difference between the tests in queryEditor.integration.spec.ts and queryEditor.spec.ts? In other words, how do we suggest to organise tests?

It's a good question!

queryEditor.spec.ts just tests the query editor. I.e checking the right endpoints are being called when a certain button is clicked and so on. I want to test the query editor in isolation, so any backend endpoints are mocked.

queryEditor.integration.spec.ts is calling the downstream sheets API and fetches data from a readonly spreadsheet.

Ultimately, it will be up to the plugin author to decide what kind of tests they want to have. Both may be useful. Hitting real downstream APIs can be very useful, but it may come with security concerns and also cause flakiness.

leventebalogh · 2023-11-24T14:15:24Z

packages/plugin-e2e/tsconfig.json

@@ -1,7 +1,8 @@
 {
 "compilerOptions": {
 "outDir": "./dist",
- "declaration": true
+ "declaration": true,
+ "stripInternal": true


Just out of curiosity - how do we decide which types should be private and which ones should be public? I sometimes find it useful during development to have access to as many types as possible when working with a package, while on the other hand it can tie our hands if we expose too much, as we can hardly change what a lot of people depend on.

Co-authored-by: Levente Balogh <balogh.levente.hu@gmail.com>

sunker · 2023-11-24T14:58:18Z

Thanks for good feedback @leventebalogh!

sunker added 7 commits November 20, 2023 17:03

add setup proj that adds data source

a12c8ff

add custom selector engine

03da826

add new fixtures and models

3ae2282

internalize certain types

f19ab91

add custom matcher for panel error

8ec6237

cleanup

e575dcf

add tests

87af085

sunker added the no-changelog Don't include in changelog and version calculations label Nov 21, 2023

sunker added 5 commits November 21, 2023 08:43

fix request

2934f45

include ds version in multi dimensional matrix

50752f3

use props ds name

1f717b0

remove example test

51be95b

use better selector

f55f80b

sunker force-pushed the query-editor branch from eaa9488 to f55f80b Compare November 21, 2023 08:24

sunker added 2 commits November 21, 2023 09:30

improve name

8f4c157

tidy up

0c02c26

sunker marked this pull request as ready for review November 21, 2023 08:48

sunker requested review from mckn, leventebalogh and jackw and removed request for mckn November 21, 2023 08:48

sunker commented Nov 21, 2023

View reviewed changes

sunker changed the title ~~Query editor~~ Plugin E2E: Make it possible to test query editor in panel edit page Nov 21, 2023

sunker added 4 commits November 21, 2023 15:49

provision ds instead of using setup proj

da2f1e0

fix env variable name

b749741

mount provisioning folder as volume

cdd4f14

remove not used file

2634e64

sunker force-pushed the query-editor branch from 580b984 to 2634e64 Compare November 21, 2023 15:25

sunker added 2 commits November 24, 2023 14:42

remove empty dashboard page

dc459ea

use correct type

c71f37e

leventebalogh approved these changes Nov 24, 2023

View reviewed changes

sunker and others added 4 commits November 24, 2023 15:40

Update packages/plugin-e2e/src/api.ts

522856a

Co-authored-by: Levente Balogh <balogh.levente.hu@gmail.com>

Update packages/plugin-e2e/src/api.ts

be1805b

Co-authored-by: Levente Balogh <balogh.levente.hu@gmail.com>

Update packages/plugin-e2e/src/api.ts

1f88ca8

Co-authored-by: Levente Balogh <balogh.levente.hu@gmail.com>

pr feedback

63f28c2

fix broken selector

49b6b9b

sunker merged commit 33000d1 into main Nov 24, 2023
18 checks passed

sunker deleted the query-editor branch November 24, 2023 18:35

sunker mentioned this pull request Dec 11, 2023

Plugin E2E: Tasklist #576

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin E2E: Make it possible to test query editor in panel edit page #551

Plugin E2E: Make it possible to test query editor in panel edit page #551

sunker commented Nov 21, 2023 •

edited

Loading

github-actions bot commented Nov 21, 2023 •

edited

Loading

sunker Nov 21, 2023

leventebalogh Nov 24, 2023

sunker Nov 24, 2023

leventebalogh left a comment

leventebalogh Nov 24, 2023 •

edited

Loading

sunker Nov 24, 2023

leventebalogh Nov 24, 2023

sunker Nov 24, 2023

leventebalogh Nov 24, 2023

sunker Nov 24, 2023

leventebalogh Nov 24, 2023

sunker Nov 24, 2023

leventebalogh Nov 24, 2023

sunker Nov 24, 2023

leventebalogh Nov 24, 2023

sunker commented Nov 24, 2023

Plugin E2E: Make it possible to test query editor in panel edit page #551

Plugin E2E: Make it possible to test query editor in panel edit page #551

Conversation

sunker commented Nov 21, 2023 • edited Loading

github-actions bot commented Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leventebalogh left a comment

Choose a reason for hiding this comment

leventebalogh Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunker commented Nov 24, 2023

sunker commented Nov 21, 2023 •

edited

Loading

github-actions bot commented Nov 21, 2023 •

edited

Loading

leventebalogh Nov 24, 2023 •

edited

Loading