Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin E2E: Pass testinfo to models #769

Merged
merged 4 commits into from
Mar 13, 2024
Merged

Conversation

sunker
Copy link
Contributor

@sunker sunker commented Feb 23, 2024

What this PR does / why we need it:

The Playwright testinfo class contains information about the currently running tests. It's passed as the second argument in tests, and the third argument in fixtures. Among many other things, testinfo allows you to conditionally fail a running test.

Today, all APIs of plugin-e2e are backwards compatible to Grafana 8.5. This is great, but I'm afraid we won't be able to commit to that for every new feature we build into plugin-e2e in the future. If for example we'd like to add support for testing Correlations at some point, we can't keep backwards compatibility as this feature was introduced pretty recently. We then need some way communicate to the user of plugin-e2e that those APIs don't work for the current Grafana version.

This PR injects the testinfo to all models. Even though we don't need it today, it will give us more flexibility when adding support for new APIs in the future. Adding testinfo to the models is a breaking change, so I rather do it now than after we've released 1.0.

Here are a few examples of how it can be used:

export class DataSourceConfigPage extends GrafanaPage {
  async addCorrelation() {
      this.ctx.testInfo.fail(
        semver.lt(this.ctx.grafanaVersion, '10.0.0'),
        'Correlations are not supported in this version of Grafana. Please skip this test for this version of Grafana.'
      );
      ...
  }
}
const createDsReq = await request.post('/api/datasources', datasource);
  if (!createDsReq.ok()) {
    this.ctx.testInfo.fail('Could not create data source');
  }

If we don't have a way to force a test to fail, it will fail on something else with possibly weird error messages that we don't control. We can write to the console, but that easily gets lost in the test report.

Breaking change notice

You now need to pass testInfo to page models.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

📦 Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @grafana/create-plugin@4.3.0-canary.769.0b01966.0
npm install @grafana/plugin-e2e@0.20.0-canary.769.0b01966.0
# or 
yarn add @grafana/create-plugin@4.3.0-canary.769.0b01966.0
yarn add @grafana/plugin-e2e@0.20.0-canary.769.0b01966.0

Copy link

github-actions bot commented Feb 23, 2024

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 sunker force-pushed the plugin-e2e/pass-testinfo-to-models branch from 044311d to d5c2920 Compare February 23, 2024 08:05
@sunker sunker added minor Increment the minor version when merged release Create a release when this pr is merged labels Feb 23, 2024
@sunker sunker requested a review from mckn February 23, 2024 08:20
@sunker sunker marked this pull request as ready for review February 23, 2024 08:20
@sunker sunker requested a review from a team as a code owner February 23, 2024 08:20
Copy link
Collaborator

@mckn mckn left a comment

Choose a reason for hiding this comment

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

LGTM!

I guess this will cover the scenario where a plugin developer is conditionally providing functionality in tier plugin based on Grafana version. So for me this makes sense. The only thing is that I think we should fail the test and let the plugin author (who writes the test) add logic to skip the test since they have added logic to conditionally use the feature/functionality.

@mckn mckn requested a review from jackw February 23, 2024 10:28
@sunker
Copy link
Contributor Author

sunker commented Feb 29, 2024

I guess this will cover the scenario where a plugin developer is conditionally providing functionality in tier plugin based on Grafana version. So for me this makes sense. The only thing is that I think we should fail the test and let the plugin author (who writes the test) add logic to skip the test since they have added logic to conditionally use the feature/functionality.

Yep totally agree!

@sunker sunker merged commit 69b5794 into main Mar 13, 2024
21 checks passed
@sunker sunker deleted the plugin-e2e/pass-testinfo-to-models branch March 13, 2024 07:37
@grafana-plugins-platform-bot
Copy link

🚀 PR was released in @grafana/plugin-e2e@0.20.0 🚀

@grafana-plugins-platform-bot grafana-plugins-platform-bot bot added the released This issue/pull request has been released. label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged release Create a release when this pr is merged released This issue/pull request has been released.
Projects
Development

Successfully merging this pull request may close these issues.

2 participants