[Devops-1004-1469] Pick flaky e2e tests and update demosetup gha #1685

mwaiyee · 2026-01-19T05:35:55Z

Pick selected flaky tests from past GHA to retry 3 times and other tests to run once only

Mount flow/base/package.json if there are changes to the file
Re-enable and fix e2e test 10-jobs/jobs-execute-view-log.spec.js

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.

Automated Tests (Jasmine integration tests, Unit tests, and/or Performance tests)
Updated Manual tests / Demo Config
Documentation (Application guide, Admin guide, Markdown, Readme and/or Wiki)
Verified that local development environment is working with latest changes (integrated with latest develop branch)
following best practices in code review doc

…ew-log.spec.js test

… mwaiyee/1004-1469

Copilot

Pull request overview

This PR consolidates multiple test improvements focused on enhancing test reliability and consistency. The changes include adding retry configurations to flaky tests, improving test naming conventions, fixing URL references, and updating the CI/CD workflow for better development environment handling.

Changes:

Added retry configuration (test.describe.configure({ retries: 3 })) to multiple E2E test files to handle flaky tests
Improved test naming consistency by adding TEST_NAME constants and SHOULD_SKIP flags to several test files
Fixed hardcoded URLs to use relative paths in test files
Updated test assertions to use dynamic values (dataset_id, schema_name) instead of hardcoded values
Modified global retry configuration in playwright.config.ts for CI environments
Enhanced GitHub workflow to handle flow base package.json changes

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/e2e/tests/17-configurations/CDM-creation.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/14-data-quality-dashboard/execute-data-quality.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/13-data-characterization/execute-data-characterization.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/12-concepts/concept-sets.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/10-jobs/jobs-execute-view-log.spec.js	Changed SHOULD_SKIP to false, fixed URL, improved test to use dynamic values, added comments
tests/e2e/tests/09-patient-analytics/patient-list.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/09-patient-analytics/filter-cards.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/09-patient-analytics/extended-query-logic.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/09-patient-analytics/cohort-entry-and-exit.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/09-patient-analytics/bookmark.spec.ts	Added retry configuration for flaky test handling
tests/e2e/tests/09-patient-analytics/atlas_cohort_definition.spec.ts	Added TEST_NAME constant and SHOULD_SKIP flag for consistency
tests/e2e/tests/08-dataflow/revert-specific-version.spec.ts	Added TEST_NAME constant and SHOULD_SKIP flag for consistency
tests/e2e/tests/05-datasets/new-schema-omop-cdm-plugin-omop54.spec.ts	Fixed semicolon formatting inconsistency
tests/e2e/tests/05-datasets/new-schema-data-management-plugin-omop.spec.ts	Reformatted indentation to use consistent 2-space indentation
tests/e2e/tests/05-datasets/dataset-overview-search.spec.ts	Added TEST_NAME, SHOULD_SKIP, and retry configuration for consistency
tests/e2e/tests/05-datasets/copy-datasets-id-schema-name.spec.js	Fixed hardcoded URL to relative path, added retry configuration
tests/e2e/playwright.config.ts	Changed CI retry configuration from 3 to 0
.github/workflows/d2e-demosetup-dev.yml	Added flow_package_json change detection and docker-compose volume mount handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-19T05:39:56Z

tests/e2e/playwright.config.ts

    screenshot: 'only-on-failure' // Take screenshot of page if test fails
  },
-  retries: process.env.CI ? 3 : 0, // retry failed tests once
+  retries: process.env.CI ? 0 : 0, // retry failed tests once


The retries configuration is being changed from 3 to 0 for CI environments. However, multiple test files are adding test.describe.configure with retries set to 3. This creates an inconsistency where individual tests configure 3 retries at the describe level, but the global CI retry setting is being reduced to 0. Consider whether the global retry setting should remain at 3 to align with the individual test retry configurations, or remove the describe-level retry configurations if the intent is to disable retries in CI.

Suggested change

retries: process.env.CI ? 0 : 0, // retry failed tests once

retries: process.env.CI ? 3 : 0, // retry failed tests up to 3 times in CI

Copilot · 2026-01-19T05:39:56Z

tests/e2e/tests/10-jobs/jobs-execute-view-log.spec.js

  await page.getByRole('button', { name: 'View Results' }).click()
  await expect(page.locator('.loading-animation-component')).not.toBeVisible()
-  await expect(page.getByTestId('dialog-title')).toHaveText(/Results for dataset: demo_cdm .+/)
+  await expect(page.getByTestId('dialog-title')).toHaveText(/Results for dataset+/)


The regex pattern has been changed from '/Results for dataset: demo_cdm .+/' to '/Results for dataset+/'. This appears to be an incomplete regex pattern. The '+' quantifier requires a preceding character or group to repeat. If the intent is to match "Results for dataset" followed by any characters, the pattern should be '/Results for dataset.+/' or '/Results for dataset:.*/' depending on whether the colon is required.

Suggested change

await expect(page.getByTestId('dialog-title')).toHaveText(/Results for dataset+/)

await expect(page.getByTestId('dialog-title')).toHaveText(/Results for dataset.+/)

Copilot · 2026-01-19T05:39:57Z

tests/e2e/tests/10-jobs/jobs-execute-view-log.spec.js

  await page
    .locator('div:nth-child(3) > .p-label__body > .schema-form-property__fields > .p-base-input > .p-textarea__control')
    .click()
+  // Fill up Schemaname field 


Missing space after the double slashes in the comment. Should be '// Fill up Schemaname field' instead of '// Fill up Schemaname field ' (note the trailing space).

Suggested change

// Fill up Schemaname field

// Fill up Schemaname field

Copilot · 2026-01-19T05:39:57Z

tests/e2e/tests/10-jobs/jobs-execute-view-log.spec.js

 const TEST_NAME = 'jobs-execute-view-log-and-result'
-const SHOULD_SKIP = true
+const SHOULD_SKIP = false
 test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)


This test file is missing the 'test.describe.configure({ retries: 3 })' configuration that was added to other test files in this PR. For consistency, consider adding this retry configuration after line 5, similar to the pattern used in other test files like CDM-creation.spec.ts, execute-data-quality.spec.ts, etc.

Suggested change

test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)

test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)

test.describe.configure({ retries: 3 })

Copilot · 2026-01-19T05:39:57Z

tests/e2e/tests/09-patient-analytics/atlas_cohort_definition.spec.ts

 import { test, expect } from '@playwright/test'
+const TEST_NAME = 'atlas-lite cohort creation'
+const SHOULD_SKIP = false
+test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)


This test file is missing the 'test.describe.configure({ retries: 3 })' configuration that was added to other test files in this PR. For consistency, consider adding this retry configuration after line 4, similar to the pattern used in other test files like patient-list.spec.ts, filter-cards.spec.ts, etc.

Suggested change

test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)

test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)

test.describe.configure({ retries: 3 })

Copilot · 2026-01-19T05:39:57Z

tests/e2e/tests/08-dataflow/revert-specific-version.spec.ts

 import { test, expect } from '@playwright/test'
+const TEST_NAME = 'dataflow-revert-specific-version'
+const SHOULD_SKIP = false
+test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)


This test file is missing the 'test.describe.configure({ retries: 3 })' configuration that was added to other test files in this PR. For consistency, consider adding this retry configuration after line 4, similar to the pattern used in other test files.

Suggested change

test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)

test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)

test.describe.configure({ retries: 3 })

mwaiyee added 3 commits January 16, 2026 18:16

1004-mount flow/base/package.json and renable 10-jobs/jobs-execute-vi…

3fa3e04

…ew-log.spec.js test

pick selected e2e tests as flaky

f394819

Merge branch 'develop' of https://github.com/OHDSI/Data2Evidence into…

b8ddeed

… mwaiyee/1004-1469

mwaiyee requested review from SantanM and csafreen as code owners January 19, 2026 05:35

Copilot AI review requested due to automatic review settings January 19, 2026 05:35

mwaiyee requested review from brandantck, p-hoffmann, satish-a0 and suwarnoong as code owners January 19, 2026 05:35

mwaiyee temporarily deployed to github January 19, 2026 05:36 — with GitHub Actions Inactive

Copilot started reviewing on behalf of mwaiyee January 19, 2026 05:36 View session

mwaiyee changed the title ~~[Devops-1004-1469] Combine PRs~~ [Devops-1004-1469] Pick flaky e2e tests and update demosetup gha Jan 19, 2026

Copilot AI reviewed Jan 19, 2026

View reviewed changes

add retry in e2e test

e786818

mwaiyee temporarily deployed to github January 19, 2026 07:36 — with GitHub Actions Inactive

add retry in e2e test

12a0069

mwaiyee temporarily deployed to github January 21, 2026 08:50 — with GitHub Actions Inactive

fix test

a54595b

mwaiyee temporarily deployed to github January 22, 2026 05:44 — with GitHub Actions Inactive

fix

658dd30

mwaiyee temporarily deployed to github January 22, 2026 05:46 — with GitHub Actions Inactive

Merge branch 'develop' into mwaiyee/1004-1469

f71951b

mwaiyee temporarily deployed to github January 23, 2026 02:44 — with GitHub Actions Inactive

satish-a0 approved these changes Jan 26, 2026

View reviewed changes

Merge branch 'develop' into mwaiyee/1004-1469

abf9ef9

mwaiyee temporarily deployed to github January 26, 2026 07:27 — with GitHub Actions Inactive

Merge branch 'develop' into mwaiyee/1004-1469

aac309f

mwaiyee temporarily deployed to github January 27, 2026 00:57 — with GitHub Actions Inactive

Merge branch 'develop' into mwaiyee/1004-1469

c2fcd4e

mwaiyee temporarily deployed to github January 27, 2026 05:34 — with GitHub Actions Inactive

mwaiyee added this pull request to the merge queue Jan 27, 2026

Merged via the queue into develop with commit 0cce0b8 Jan 27, 2026
32 checks passed

mwaiyee deleted the mwaiyee/1004-1469 branch January 27, 2026 08:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Devops-1004-1469] Pick flaky e2e tests and update demosetup gha #1685

[Devops-1004-1469] Pick flaky e2e tests and update demosetup gha #1685

Uh oh!

mwaiyee commented Jan 19, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	retries: process.env.CI ? 0 : 0, // retry failed tests once
	retries: process.env.CI ? 3 : 0, // retry failed tests up to 3 times in CI

	await expect(page.getByTestId('dialog-title')).toHaveText(/Results for dataset+/)
	await expect(page.getByTestId('dialog-title')).toHaveText(/Results for dataset.+/)

	test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)
	test.fixme(SHOULD_SKIP, `${TEST_NAME} test is temporarily disabled.`)
	test.describe.configure({ retries: 3 })

[Devops-1004-1469] Pick flaky e2e tests and update demosetup gha #1685

[Devops-1004-1469] Pick flaky e2e tests and update demosetup gha #1685

Uh oh!

Conversation

mwaiyee commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mwaiyee commented Jan 19, 2026 •

edited

Loading