-
Notifications
You must be signed in to change notification settings - Fork 1
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
Bohandley/integration tests #27
Changes from 60 commits
ca63d5d
7bb3bb9
7fa95b1
6eb91cb
8c87d93
748f079
2a40343
8c6c624
ba04e71
4cb61f0
b7d7691
987f7c9
dd9b24b
93255e4
7f3c6d7
d9f56b0
0141329
55ff0cb
514ecf4
b7865c3
e0e3cbe
6506c6f
fb73140
3a80071
2c6f5fa
34b0c1d
78d3108
55264de
4a43717
5109b31
9b47674
4bd1757
3a47d70
7580e27
4d60ae0
b27af6f
1441c9d
6dfcbb7
b8b4c57
d36b942
ec5bc4e
ece4d73
6ce7ebf
a5275b1
01724f4
d37f5d1
c1d974e
ccce54e
27b4f8a
ed88f55
685f3b1
bf4d7fa
60cff29
014ed0c
3d3c3e3
53cb73e
79b889a
64e2497
2acd402
356176b
297860e
0576175
9412e6a
b68ceb6
8e3b0cd
62955e1
e6cb8bf
6af3010
673e9b8
b86b09a
9b7df1a
ef48b54
f53f785
6440d59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
name: E2E Tests | ||
on: | ||
pull_request: | ||
paths-ignore: | ||
- 'docs/**' | ||
push: | ||
paths-ignore: | ||
- 'docs/**' | ||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} | ||
cancel-in-progress: true | ||
jobs: | ||
test: | ||
timeout-minutes: 60 | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Checkout | ||
uses: actions/checkout@v3 | ||
- name: Setup Go environment | ||
uses: actions/setup-go@v4 | ||
with: | ||
go-version: '1.21.6' | ||
# NOT TESTING THE BACKEND YET | ||
# - name: Test backend | ||
# uses: magefile/mage-action@v2 | ||
# with: | ||
# version: latest | ||
# args: coverage | ||
- name: Build backend | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Building the backend here @itsmylife |
||
uses: magefile/mage-action@v2 | ||
with: | ||
version: latest | ||
args: buildAll | ||
- name: Setup Node | ||
uses: actions/setup-node@v3 | ||
with: | ||
node-version: 20 | ||
- name: Install dependencies | ||
run: npm install -g yarn && yarn install --frozen-lockfile; | ||
- name: Build Frontend | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Building the frontend here @itsmylife |
||
run: | | ||
yarn build; | ||
- name: Start the docker container for E2E | ||
run: | | ||
docker compose -f docker-compose-debug.yaml pull | ||
docker compose -f docker-compose-debug.yaml up -d | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly, this PR only tests your plugin against the latest enterprise main build. I see you have specified grafana 10.4.0 as min dep in the plugin.json. If the plugin is supposed to be compatible with all grafana versions beyond 10.4.0, I suggest you start using the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is awesome! We still need to decide what versions we will support so I will add this to an issue to update the workflow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
- name: Wait for prometheus to start | ||
uses: nev7n/wait_for_response@v1 | ||
with: | ||
url: 'http://localhost:9090/-/ready' | ||
responseCode: 200 | ||
timeout: 20000 | ||
interval: 500 | ||
- name: Wait for grafana to start | ||
uses: nev7n/wait_for_response@v1 | ||
with: | ||
url: 'http://localhost:3099/' | ||
responseCode: 200 | ||
timeout: 20000 | ||
interval: 500 | ||
- name: Wait for prometheus to be configured in grafana | ||
uses: nev7n/wait_for_response@v1 | ||
with: | ||
url: 'http://localhost:3099/api/datasources/uid/prometheus-amazon/health' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't just waiting for this url be enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what you mean? Waiting for this response ensures that the Grafana can make a request to Prometheus and the response is a 200. This might not be necessary though, waiting for Prometheus and Grafana might be enough. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I mean if this url returns 200, it means Grafana and Prometheus are running so we can remove those steps in the workflow? |
||
responseCode: 200 | ||
timeout: 20000 | ||
interval: 500 | ||
- name: Install Playwright Browsers | ||
run: yarn playwright install --with-deps | ||
- name: Run E2E tests | ||
run: yarn test:e2e | ||
- name: Run E2E report | ||
uses: actions/upload-artifact@v3 | ||
if: always() | ||
with: | ||
name: playwright-report | ||
path: playwright-report/ | ||
retention-days: 30 | ||
- name: Run E2E videos | ||
uses: actions/upload-artifact@v3 | ||
if: always() | ||
with: | ||
name: test-results | ||
path: test-results/ | ||
retention-days: 30 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Downloading the report manually and firing it up locally is pretty time consuming. The example workflow for a backend ds plugin in this guide has a step that pushes the playwright report to a GCS bucket. It also drops a link to the report in the workflow summary, so you can browse the report directly from the PR without having to download it manually. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is super helpful. We will have to choose a storage location appropriate for the team that will take this repository on, so I will add this as an issue to improve the debugging experience. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If you use the |
||
- name: Stop the docker container | ||
if: always() | ||
run: docker-compose -f docker-compose-debug.yaml down |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,4 +32,8 @@ e2e-results/ | |
# Editor | ||
.idea | ||
|
||
.eslintcache | ||
.eslintcache | ||
/test-results/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest you uncomment this - there's not reason to version control you local generated reports. |
||
# /playwright-report/ | ||
/blob-report/ | ||
/playwright/.cache/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
version: "3.7" | ||
services: | ||
prometheus: | ||
image: prom/prometheus:v2.45.0 | ||
container_name: prometheus-debug | ||
command: | ||
- '--config.file=/etc/prometheus/prometheus.yml' | ||
ports: | ||
- 9090:9090 | ||
restart: unless-stopped | ||
volumes: | ||
- ./prometheus:/etc/prometheus | ||
- prom_data:/prometheus | ||
grafana: | ||
image: grafana/grafana-enterprise:${GF_VERSION:-main} | ||
container_name: grafana-prometheus-amazon-debug | ||
ports: | ||
- 3099:3099 | ||
restart: unless-stopped | ||
environment: | ||
- GF_DEFAULT_APP_MODE=development | ||
- GF_AUTH_ANONYMOUS_ORG_ROLE=Admin | ||
- GF_AUTH_ANONYMOUS_ENABLED=true | ||
- GF_AUTH_BASIC_ENABLED=false | ||
- GF_SERVER_HTTP_PORT=3099 | ||
volumes: | ||
- ./provisioning/datasources:/etc/grafana/provisioning/datasources | ||
- ./dist:/var/lib/grafana/plugins/prometheus-amazon | ||
depends_on: | ||
prometheus: | ||
condition: service_started | ||
volumes: | ||
prom_data: |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
version: '3.0' | ||
version: '3.7' | ||
|
||
services: | ||
grafana: | ||
|
@@ -12,4 +12,4 @@ services: | |
- 3000:3000/tcp | ||
volumes: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh! That docker file is not what we use to test things currently, but it comes "out of the box" with a backend plugin. The docker compose file with all the nice things is |
||
- ./dist:/var/lib/grafana/plugins/prometheus-amazon-datasource | ||
- ./provisioning:/etc/grafana/provisioning | ||
- ./provisioning/datasources:/etc/grafana/provisioning |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import { DataSourcePluginOptionsEditorProps } from '@grafana/data'; | ||
import { selectors } from '@grafana/e2e-selectors'; | ||
import { test, expect } from '@grafana/plugin-e2e'; | ||
import { PromOptions } from '@grafana/prometheus'; | ||
|
||
test.describe('Prometheus annotation query editor', () => { | ||
test('Check that the editor uses the code editor', async ({ | ||
readProvisionedDataSource, | ||
annotationEditPage | ||
}) => { | ||
const ds = await readProvisionedDataSource<DataSourcePluginOptionsEditorProps<PromOptions>>({ fileName: 'datasources.yml' }); | ||
|
||
await annotationEditPage.datasource.set(ds.name); | ||
await annotationEditPage | ||
.getByTestIdOrAriaLabel(selectors.components.DataSource.Prometheus.queryEditor.code.queryField).focus(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't think you need this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right, I don't think I need the focus or setting the DS, because the default DS is Prometheus. |
||
|
||
await expect(annotationEditPage | ||
.getByTestIdOrAriaLabel(selectors.components.DataSource.Prometheus.queryEditor.code.queryField)).toBeVisible(); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth updating to
actions/checkout@v4
,actions/setup-go@v5
, andactions/setup-node@v4
. The latest versions of the Go and Node actions have the most capabilities with respect to dependency caching, which can significantly speed up CI/CD.For more info:
An important difference between the latest Go & Node actions is that the Go one enables caching by default, and the Node one does not - it requires a few lines of config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, I'm going to add this as an issue to improve CI/CD for this repo!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go caching is working! Saved 6 minutes of workflow time. I'm a little stuck on the node caching currently but it is captured in the issue.