E2E: Improve support to interact with panel edit options

[mckn]

What this PR does / why we need it:
This PR introduce an abstraction to interact with the panel edit options. This is useful when developing panel plugins and you want to verify that different combinations of settings works as expected. The goal is to make it easier and more robust to write tests that interacts with the panel editor. This will also give us a layer where we can cater for differences in the panel editor between Grafana versions.

One example of this is the recent change of the switch component that changed role from checkbox to switch. Developers writing tests interacting with the switch directly would need to cater for this in the test which will make it harder and more complex to test your plugin against multiple versions of Grafana.

Note: A more robust way to test this is to provision one panel per setting. So you never interact with the panel editor while running tests. You only test that the setting has the expected behaviour on your panel. This might not be possible in all scenarios e.g. when adding a custom panel editor or having cascading settings.

This is how the configuration of the panel edit options map into the test APIs.

• addNumberInput -> getNumberInput
• addSliderInput -> getSliderInput
• addTextInput -> getTextInput
• addSelect -> getSelect()
• addMultiSelect -> getMultiSelect()
• addRadio -> getRadio()
• addBooleanSwitch -> getSwitch
• addColorPicker -> getColorPicker
• addTimeZonePicker -> getSelect
• addUnitPicker -> getUnitPicker
• addFieldNamePicker -> getSelect
• addDashboardPicker -> getSelect

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:
This is a suggestion and I would like to get more eyes on this to get better naming, structure and ways to interact with the panel editor in your test.

On thing that isn't that great is that while writing the tests you use the "labels" visible in the UI to select options to interact with. This is great for the options that the plugin adds because the plugin controls the labels that they add. But for the standard options or options that is added by Grafana we can still run into the problem that a plugin developer writes a test and interacts with the Standard options label. If we, for some reason, change that in a Grafana version the tests will stop working and the tests will not be able to run across multiple versions of Grafana.

📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install [email protected]
npm install @grafana/[email protected]
npm install @grafana/[email protected]
npm install @grafana/[email protected]
npm install @grafana/[email protected]
npm install @grafana/[email protected]
# or 
yarn add [email protected]
yarn add @grafana/[email protected]
yarn add @grafana/[email protected]
yarn add @grafana/[email protected]
yarn add @grafana/[email protected]
yarn add @grafana/[email protected]

[github-actions]

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

✨ This PR can be merged and will trigger a new minor release.
NOTE: When merging a PR with the release label please avoid merging another PR. For further information see here.

[sunker]

Nice! I think I prefer option 1 as option 2 would make us slightly more vulnerable to changes in the core Playwright api.

Question: Would it be better to replace input and switch with getByRole(role: 'input' | 'switch' | ..., { name: string }: Options)? This would make our abstraction look closer to the playwright api?

const group = panelEditPage.getOptionsGroup('Clock panel')
await group.getByRole('switch', { name: 'enableClock' }).uncheck()

[mckn]

Nice! I think I prefer option 1 as option 2 would make us slightly more vulnerable to changes in the core Playwright api.

Question: Would it be better to replace input and switch with getByRole(role: 'input' | 'switch' | ..., { name: string }: Options)? This would make our abstraction look closer to the playwright api?

const group = panelEditPage.getOptionsGroup('Clock panel')
await group.getByRole('switch', { name: 'enableClock' }).uncheck()

Yeah, but on the other hand 'input' is not a role. In the scenario where with the switch-bug you would still need to know if you are trying to fetch a combobox or a switch role.

[mckn]

@sunker I added some more code and tests. Lets have a sync about this before I continue further but it feels kind of OK to work with when writing tests.

[sunker]

Nice! Great tests!

I had a quick look at the helper methods in the panel options API and this is what you can do:

addNestedOptions (can skip this for now)
addNumberInput (textbox)
addSliderInput (not sure, textbox I guess?)
addTextInput (textbox or textarea)
addSelect (combobox)
addMultiSelect (combobox)
addRadio (radiogroup)
addBooleanSwitch (switch in recent versions) 
addColorPicker (don't know - can probably skip for now)
addTimeZonePicker (combobox)
addUnitPicker (comobobox)
addFieldNamePicker (combobox)
addDashboardPicker (combobox) 

So basically we need to support textbox, textarea, combobox, radiogroup and switch. So I guess we'll have to choose between:

group.getByRole(role: textbox | textarea | combobox | radiogroup | switch, options: { ... })
// or implementing each specifically like in this PR. e.g
group.getRadioButtonGroup(label: string)

I don't have strong opinions here. The tricky bit will be to get them to work in >= 8.5.0.

[mckn]

Nice! Great tests!

I had a quick look at the helper methods in the panel options API and this is what you can do:

addNestedOptions (can skip this for now)
addNumberInput (textbox)
addSliderInput (not sure, textbox I guess?)
addTextInput (textbox or textarea)
addSelect (combobox)
addMultiSelect (combobox)
addRadio (radiogroup)
addBooleanSwitch (switch in recent versions) 
addColorPicker (don't know - can probably skip for now)
addTimeZonePicker (combobox)
addUnitPicker (comobobox)
addFieldNamePicker (combobox)
addDashboardPicker (combobox) 

So basically we need to support textbox, textarea, combobox, radiogroup and switch. So I guess we'll have to choose between:

group.getByRole(role: textbox | textarea | combobox | radiogroup | switch, options: { ... })
// or implementing each specifically like in this PR. e.g
group.getRadioButtonGroup(label: string)

I don't have strong opinions here. The tricky bit will be to get them to work in >= 8.5.0.

I'm happy with both as well. The only thing I feel is a bit negative with the getByRole is that it diverge from the standard getByRole since we are introducing "new" roles.

But I'm happy to proceed with that approach as well. Maybe ask for some more input here?

[sunker]

The only thing I feel is a bit negative with the getByRole is that it diverge from the standard getByRole since we are introducing "new" roles.

You mean because they only make up a subset of all roles that exists?

Maybe ask for some more input here?

Sounds good! Maybe Jack? He's affected by the recent change in his traffic light panel. :)

[mckn]

You mean because they only make up a subset of all roles that exists?

Yes, and we would need to have custom roles that match to our components. E.g. in the scenario with the recent change the switch got changed from a 'checkbox' to a 'switch' and in those cases we would not want the developer to need to update their tests. In case they have getByRole('checkbox', ...);.

I understood your suggestion as we don't use the default roles but use our own that maps to our components rather than the html roles. Otherwise we don't resolve issues as the one with the switch.

[sunker]

Great stuff! From my standpoint we're ready to ship this, but I think we should try to add proper selector for a few of the elements in Grafana first.

[sunker]

now that all tests pass, we can revert the change in this file :)

[mckn]

Everything is fixed except that I have not updated the dependency on the canary package and hence not using the selector in the ColorPicker. Will do that asap.

[sunker]

Great stuff!! 🕺

I think what's left to do know is:

• update all docs and snippets so that they use the new awesome API
• add api-test in grafana/grafana to verify the new APIs won't break
• update create-plugin scaffolding so that the new APIs are used
• update examples repo to use the new APIs

It's totally fine with me to do this later in follow up PRs, but the docs should probably be addressed quickly after this PR is merged.

Awesome work on this!

[grafana-plugins-platform-bot]

🚀 PR was released in @grafana/[email protected] 🚀