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

re-enable e2e UI tests on CI #1961

Merged
merged 19 commits into from
May 23, 2023
Merged

Conversation

joeyorlando
Copy link
Contributor

@joeyorlando joeyorlando commented May 17, 2023

#1692 is still open. This PR is not an ideal approach, but it's a quick win while we wait for that issue to be resolved.

By retrying failing tests up to 3 times, we should be fine to re-enable these on CI. If a test is failing > 3 times, there's likely a legitimate issue occuring.

@joeyorlando joeyorlando added pr:no changelog pr:no public docs Added to a PR that does not require public documentation updates labels May 17, 2023
@joeyorlando joeyorlando requested review from a team May 17, 2023 18:54
@joeyorlando joeyorlando force-pushed the jorlando/turn-e2e-tests-back-on-ci branch 2 times, most recently from 9e80c76 to 3b94acb Compare May 17, 2023 20:39
Copy link
Contributor Author

Choose a reason for hiding this comment

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

component is not referenced in either repo

runs-on: ubuntu-latest
name: "End to end tests - Grafana: ${{ matrix.grafana-image-tag }}"
strategy:
matrix:
grafana-image-tag:
- 8.5.22
Copy link
Contributor Author

Choose a reason for hiding this comment

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

OnCall does not work on this version and we may want to consider updating the dependencies object in the plugin.json to reflect this:
Screenshot 2023-05-17 at 19 25 02

@joeyorlando joeyorlando mentioned this pull request May 18, 2023
10 tasks
@@ -620,8 +620,7 @@
}
],
"dependencies": {
"grafanaDependency": ">=8.3.2",
"grafanaVersion": "8.3",
Copy link
Contributor Author

@joeyorlando joeyorlando May 18, 2023

Choose a reason for hiding this comment

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

dependencies.grafanaVersion is deprecated according to the docs

- run on several more grafana versions
- update grafanaDependency version in plugin.json
@joeyorlando joeyorlando force-pushed the jorlando/turn-e2e-tests-back-on-ci branch from 4dac1fc to cb82a75 Compare May 18, 2023 22:31
@@ -14,7 +14,7 @@ const config: PlaywrightTestConfig = {
testDir: './integration-tests',
globalSetup: './integration-tests/globalSetup.ts',
/* Maximum time one test can run for. */
timeout: 90 * 1000,
timeout: 60 * 1000,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

60s should be plenty of time for most tests, if we put this timeout too long, it'll take forever for the CI job to fail (especially with retries enabled).

There is currently one test, "we can create an oncall schedule + receive an alert", that genuinely takes longer (because it creates a ton of different objects). This test has already been marked w/ test.slow() which increases the timeout value by 3x just for this test.

@joeyorlando joeyorlando reopened this May 22, 2023
Comment on lines +271 to +273
# - 8.5.22
# - 9.0.0
# - 9.1.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Ukochka will your PR (#1800) fix these issues? If so, you should consider turning on the tests for these versions in your PR

@@ -408,8 +423,7 @@ jobs:
GRAFANA_PASSWORD: oncall
MAILSLURP_API_KEY: ${{ secrets.MAILSLURP_API_KEY }}
working-directory: ./grafana-plugin
# -x = exit command after first failing test
run: yarn test:integration -x
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't exit on first failed test as this defeats the purpose of allowing the retries to get around tests which are currently flaky

@joeyorlando joeyorlando enabled auto-merge May 23, 2023 21:25
@joeyorlando joeyorlando disabled auto-merge May 23, 2023 21:26
@joeyorlando joeyorlando merged commit c793e55 into dev May 23, 2023
@joeyorlando joeyorlando deleted the jorlando/turn-e2e-tests-back-on-ci branch May 23, 2023 21:26
brojd pushed a commit that referenced this pull request Sep 18, 2024
#1692 is still open. This PR is not an ideal approach, but it's a quick
win while we wait for that issue to be resolved.

By retrying failing tests up to 3 times, we _should_ be fine to
re-enable these on CI. If a test is failing > 3 times, there's likely a
legitimate issue occuring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants