[Devops-1469] Pick selected e2e tests as flaky #1506
Conversation
mwaiyee
requested review from
SantanM,
csafreen,
p-hoffmann,
satish-a0 and
suwarnoong
as code owners
There was a problem hiding this comment.
Pull request overview
This PR implements selective retry logic for flaky e2e tests by disabling global retries in CI and configuring specific tests to retry up to 3 times. The changes also include URL standardization and code formatting improvements.
Key Changes:
- Disabled global CI retries in Playwright configuration (from 3 to 0)
- Added per-test retry configuration using
test.describe.configure({ retries: 3 })for 7 identified flaky tests - Standardized test URLs by replacing hardcoded URLs with relative paths
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/playwright.config.ts | Changed global CI retry from 3 to 0 to disable automatic retries except for specifically marked tests |
| tests/e2e/tests/14-data-quality-dashboard/execute-data-quality.spec.ts | Added retry configuration for flaky data quality test |
| tests/e2e/tests/09-patient-analytics/patient-list.spec.ts | Added retry configuration for flaky patient list test |
| tests/e2e/tests/09-patient-analytics/bookmark.spec.ts | Added retry configuration for flaky bookmark test |
| tests/e2e/tests/09-patient-analytics/atlas_cohort_definition.spec.ts | Added retry configuration and standardized test structure with TEST_NAME constant |
| tests/e2e/tests/08-dataflow/revert-specific-version.spec.ts | Added retry configuration and standardized test structure with TEST_NAME constant |
| tests/e2e/tests/05-datasets/new-schema-omop-cdm-plugin-omop54.spec.ts | Added retry configuration and fixed semicolon formatting |
| tests/e2e/tests/05-datasets/new-schema-data-management-plugin-omop.spec.ts | Added retry configuration and reformatted indentation for consistency |
| tests/e2e/tests/05-datasets/copy-datasets-id-schema-name.spec.js | Replaced hardcoded URL with relative path |
| tests/e2e/tests/10-jobs/jobs-execute-view-log.spec.js | Replaced hardcoded URL with relative path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const TEST_NAME = 'dataflow-revert-specific-version' | ||
| const SHOULD_SKIP = false | ||
| test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`) | ||
| test.describe.configure({ retries: 3 }) // Retry up to 3 times for flaky tests |
There was a problem hiding this comment.
The test.describe.configure() method must be called within a test.describe() block to work properly. When used at the file level without a describe block, it won't have any effect on the test retry behavior. You need to wrap your test in a describe block first.
| const TEST_NAME = 'dataset-new-schema-omop-cdm-plugin-54' | ||
| const SHOULD_SKIP = false | ||
| test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`) | ||
| test.describe.configure({ retries: 3 }) // Retry up to 3 times for flaky tests |
There was a problem hiding this comment.
The test.describe.configure() method must be called within a test.describe() block to work properly. When used at the file level without a describe block, it won't have any effect on the test retry behavior. You need to wrap your test in a describe block first.
| const TEST_NAME = 'dataset-new-schema-data-management-plugin-omop' | ||
| const SHOULD_SKIP = false | ||
| test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`) | ||
| test.describe.configure({ retries: 3 }) // Retry up to 3 times for flaky tests |
There was a problem hiding this comment.
The test.describe.configure() method must be called within a test.describe() block to work properly. When used at the file level without a describe block, it won't have any effect on the test retry behavior. You need to wrap your test in a describe block first.
| await context.grantPermissions(['clipboard-read', 'clipboard-write']) | ||
|
|
||
| await page.goto(`https://localhost:443/d2e`) | ||
| await page.goto('/d2e/portal') |
There was a problem hiding this comment.
The URL path should be '/d2e/portal' to be consistent with other test files and ensure proper navigation to the portal login page. The current path '/d2e' may not route correctly.
| const TEST_NAME = 'execute-data-quality' | ||
| const SHOULD_SKIP = false | ||
| test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`) | ||
| test.describe.configure({ retries: 3 }) // Retry up to 3 times for flaky tests |
There was a problem hiding this comment.
The test.describe.configure() method must be called within a test.describe() block to work properly. When used at the file level without a describe block, it won't have any effect on the test retry behavior. You need to wrap your test in a describe block first.
| const TEST_NAME = 'patient-analytics-patient-list' | ||
| const SHOULD_SKIP = false | ||
| test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`) | ||
| test.describe.configure({ retries: 3 }) // Retry up to 3 times for flaky tests |
There was a problem hiding this comment.
The test.describe.configure() method must be called within a test.describe() block to work properly. When used at the file level without a describe block, it won't have any effect on the test retry behavior. You need to wrap your test in a describe block first.
| const TEST_NAME = 'patient_analytics_bookmark' | ||
| const SHOULD_SKIP = false | ||
| test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`) | ||
| test.describe.configure({ retries: 3 }) // Retry up to 3 times for flaky tests |
There was a problem hiding this comment.
The test.describe.configure() method must be called within a test.describe() block to work properly. When used at the file level without a describe block, it won't have any effect on the test retry behavior. You need to wrap your test in a describe block first.
| const TEST_NAME = 'atlas-lite cohort creation' | ||
| const SHOULD_SKIP = false | ||
| test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`) | ||
| test.describe.configure({ retries: 3 }) // Retry up to 3 times for flaky tests |
There was a problem hiding this comment.
The test.describe.configure() method must be called within a test.describe() block to work properly. When used at the file level without a describe block, it won't have any effect on the test retry behavior. You need to wrap your test in a describe block first.
Signed-off-by: mwaiyee <waiyee.man@data4life-asia.care>
Merge Checklist
Please cross check this list if additions / modifications needs to be done on top of your core changes and tick them off. Reviewer can as well glance through and help the developer if something is missed out.
developbranch)