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

chore: initial e2e tests #285

Merged
merged 2 commits into from
Mar 6, 2024
Merged

Conversation

tracy-french
Copy link
Collaborator

@tracy-french tracy-french commented Feb 28, 2024

What this PR does / why we need it: This change includes the initial e2e test setup and a confirmation test for the "Get property value" query (see #212). The initial implementation does not rely on a provisioned data source and instead mocks network requests.

The change also includes a GitHub action for running the tests on PR push (following guidance int Grafana testing guide).

Follow-up PRs will include:

  • Tests for all query types (including non-happy path tests)
  • Tests for the config editor
  • Tests for the asset browser
  • Increased test writing scalability (e.g., mocking requests, test fixtures, utilities)
  • True e2e tests using real SiteWise resources and services

The test added assumes the fix in #286 is merged to test asset property selection.

@tracy-french tracy-french force-pushed the chore-e2e-tests branch 2 times, most recently from 92b0621 to f7d1465 Compare February 28, 2024 22:39
@tracy-french tracy-french changed the title chore: e2e tests chore: initial e2e tests Feb 28, 2024
@tracy-french tracy-french marked this pull request as ready for review February 28, 2024 22:49
@tracy-french tracy-french requested a review from a team as a code owner February 28, 2024 22:49
@tracy-french tracy-french requested review from iwysiu and sarahzinger and removed request for a team February 28, 2024 22:49
@tracy-french tracy-french force-pushed the chore-e2e-tests branch 3 times, most recently from 4a302d6 to 3a06ed7 Compare February 29, 2024 21:50
@tracy-french
Copy link
Collaborator Author

Updated PR with GH action.

Copy link
Contributor

@kevinwcyu kevinwcyu left a comment

Choose a reason for hiding this comment

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

@tracy-french, awesome work!

Most of the comments are just used to mark a place in the code to apply a preceding suggestion, so most of the comments are just duplicates.

Thanks for helping us get the initial e2e tests setup. I think you may be one of the first (possibly the first person) outside of Grafana to try out the new @grafana/plugin-e2e package. Hopefully you found the docs helpful. As @grafana/plugin-e2e and its docs are still a WIP, we'd welcome any feedback about it if you have any.

.github/workflows/e2e.yml Outdated Show resolved Hide resolved
.github/workflows/e2e.yml Outdated Show resolved Hide resolved
.github/workflows/e2e.yml Show resolved Hide resolved
Comment on lines 76 to 80
- name: Upload artifacts
uses: actions/upload-artifact@v4
if: ${{ (always() && steps.run-tests.outcome == 'success') || (failure() && steps.run-tests.outcome == 'failure') && github.event.organization.login != 'grafana' }}
with:
name: playwright-report-${{ matrix.GRAFANA_IMAGE.NAME }}-v${{ matrix.GRAFANA_IMAGE.VERSION }}-${{github.run_id}}
path: playwright-report/
retention-days: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Upload artifacts
uses: actions/upload-artifact@v4
if: ${{ (always() && steps.run-tests.outcome == 'success') || (failure() && steps.run-tests.outcome == 'failure') && github.event.organization.login != 'grafana' }}
with:
name: playwright-report-${{ matrix.GRAFANA_IMAGE.NAME }}-v${{ matrix.GRAFANA_IMAGE.VERSION }}-${{github.run_id}}
path: playwright-report/
retention-days: 30
- name: Publish report to GCS
if: ${{ (always() && steps.run-tests.outcome == 'success') || (failure() && steps.run-tests.outcome == 'failure') && github.event.organization.login == 'grafana' }}
uses: grafana/plugin-actions/publish-report@main
with:
grafana-version: ${{ matrix.GRAFANA_IMAGE.VERSION }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like an issue with the GH action being run from a fork. I've asked for some clarification on this from the publish report action author. I'll let you know what the suggested action when they get back to me.

tests/data-source.setup.ts Outdated Show resolved Hide resolved
tests/queryEditor.spec.ts Outdated Show resolved Hide resolved
tests/queryEditor.spec.ts Outdated Show resolved Hide resolved
tests/queryEditor.spec.ts Show resolved Hide resolved
tests/queryEditor.spec.ts Outdated Show resolved Hide resolved
tests/queryEditor.spec.ts Outdated Show resolved Hide resolved
@tracy-french
Copy link
Collaborator Author

@kevinwcyu Thank you for your time and feedback! I will work on addressing all of your comments shortly!

Exciting to be one of the first using @grafana/plugin-e2e. I'll write up a few of my thoughts about the package and drop them here for you.

@tracy-french tracy-french force-pushed the chore-e2e-tests branch 9 times, most recently from 9caef18 to a21231f Compare March 4, 2024 19:05
@tracy-french
Copy link
Collaborator Author

@kevinwcyu All of your feedback has been addressed. Just running into the issues with the dev image and GCS.

Copy link
Contributor

@kevinwcyu kevinwcyu left a comment

Choose a reason for hiding this comment

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

LGTM. Tentatively approving as it should be ready once the docker-compose.yaml file is updated and we figure out what to do with the publish report step (this will be on our end to figure out).

Nice catch on the Format property having an incorrect id.

.github/workflows/e2e.yml Outdated Show resolved Hide resolved
Comment on lines 76 to 80
- name: Upload artifacts
uses: actions/upload-artifact@v4
if: ${{ (always() && steps.run-tests.outcome == 'success') || (failure() && steps.run-tests.outcome == 'failure') && github.event.organization.login != 'grafana' }}
with:
name: playwright-report-${{ matrix.GRAFANA_IMAGE.NAME }}-v${{ matrix.GRAFANA_IMAGE.VERSION }}-${{github.run_id}}
path: playwright-report/
retention-days: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like an issue with the GH action being run from a fork. I've asked for some clarification on this from the publish report action author. I'll let you know what the suggested action when they get back to me.

@tracy-french
Copy link
Collaborator Author

@kevinwcyu As the tests are now running on every version and we just have the GCS action left to handle, I'm going to merge this. Maybe we'll see the action succeed 🤞.

@tracy-french tracy-french merged commit 673e0ac into grafana:main Mar 6, 2024
4 of 11 checks passed
@tracy-french tracy-french deleted the chore-e2e-tests branch March 6, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants