-
Notifications
You must be signed in to change notification settings - Fork 652
e2e components tests: speed up tests execution #31379
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
base: 25_2
Are you sure you want to change the base?
e2e components tests: speed up tests execution #31379
Conversation
4c27b16 to
6a3cd89
Compare
679aefd to
d7d08a3
Compare
b938314 to
bfff62b
Compare
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.
Pull Request Overview
This PR reorganizes the e2e test infrastructure by moving Gantt, FilterBuilder, and Pagination tests to a common folder structure, and implements dynamic script loading to optimize test execution. The changes include relocating test files, updating import paths, and introducing conditional loading of external dependencies (Quill, Gantt, ASP.NET Data) based on test metadata.
Reviewed Changes
Copilot reviewed 44 out of 159 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/testcafe-models/dataGrid/index.ts | Updated scrollBy method signature to require TestController parameter and add scrollable check |
| e2e/testcafe-devextreme/tests/scheduler/common/grouping/groupHeaderLongNamesCss.ts | Added debugging console.log statements to output page HTML |
| e2e/testcafe-devextreme/tests/scheduler/common/api/resourceRequestCount.ts | Added loadAspNetData metadata to tests requiring ASP.NET data |
| e2e/testcafe-devextreme/tests/scheduler/common/agenda/keyField.ts | Added loadAspNetData metadata to tests requiring ASP.NET data |
| e2e/testcafe-devextreme/tests/editors/selectBox/common.ts | Added loadAspNetData metadata to test requiring ASP.NET data |
| e2e/testcafe-devextreme/tests/editors/htmlEditor/*.ts | Added loadQuill metadata to tests requiring Quill library |
| e2e/testcafe-devextreme/tests/editors/chat/messageList.ts | Added loadAspNetData metadata to test requiring ASP.NET data |
| e2e/testcafe-devextreme/tests/dataGrid/**/*.ts | Updated all scrollBy calls to include TestController parameter, added loadAspNetData metadata where needed |
| e2e/testcafe-devextreme/tests/container.html | Commented out static script loading for dx-quill, dx-gantt, dx.all, and dx.aspnet.data |
| e2e/testcafe-devextreme/tests/common/pagination/*.ts | Moved from tests/pagination/ and updated import paths |
| e2e/testcafe-devextreme/tests/common/gantt/*.ts | Moved from tests/gantt/ and updated import paths, added loadGantt metadata |
| e2e/testcafe-devextreme/tests/common/filterBuilder/*.ts | Moved from tests/filterBuilder/ and updated import paths |
| e2e/testcafe-devextreme/tests/cardView/headerFilter/remote.functional.ts | Added loadAspNetData metadata to tests requiring ASP.NET data |
| e2e/testcafe-devextreme/tests/accessibility/*.ts | Added loadQuill metadata and updated import paths |
| e2e/testcafe-devextreme/runner.ts | Implemented dynamic script loading logic based on test metadata, added debugging logs, commented out retry logic |
| e2e/testcafe-devextreme/helpers/themeUtils.ts | Refactored screenshot naming logic for better clarity |
| e2e/testcafe-devextreme/helpers/testPageUtils.ts | Added functions for dynamic loading of Quill, Gantt, ASP.NET Data, and DevExtreme scripts, fixed typo in comment |
| e2e/testcafe-devextreme/helpers/safeSizeTest.ts | Added support for test metadata in safeSizeTest function |
| e2e/testcafe-devextreme/helpers/accessibility/test.ts | Added support for test metadata in accessibility testing |
| .github/workflows/testcafe_tests.yml | Commented out treeList and pivotGrid test configurations |
| const pageHTML = await t.eval(() => document.documentElement.outerHTML); | ||
| console.log('=== 2FULL PAGE HTML ==='); | ||
| console.log(pageHTML); | ||
|
|
||
| const bodyHTML = await t.eval(() => document.body.innerHTML); | ||
| console.log('=== 2BODY HTML ==='); | ||
| console.log(bodyHTML); |
Copilot
AI
Oct 20, 2025
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.
Debug console.log statements should be removed before merging to production. These logging statements appear to be temporary debugging code that was left in the test.
| console.log('=== FULL PAGE HTML ==='); | ||
| console.log(pageHTML); | ||
|
|
||
| const bodyHTML = await t.eval(() => document.body.innerHTML); | ||
| console.log('=== BODY HTML ==='); | ||
| console.log(bodyHTML); |
Copilot
AI
Oct 20, 2025
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.
Debug console.log statements should be removed before merging to production. These logging statements appear to be temporary debugging code that was left in the test before hook.
| console.log('=== FULL PAGE HTML ==='); | |
| console.log(pageHTML); | |
| const bodyHTML = await t.eval(() => document.body.innerHTML); | |
| console.log('=== BODY HTML ==='); | |
| console.log(bodyHTML); | |
| const bodyHTML = await t.eval(() => document.body.innerHTML); |
| // const LAUNCH_RETRY_ATTEMPTS = 5; | ||
| // const LAUNCH_RETRY_TIMEOUT = 20000; |
Copilot
AI
Oct 20, 2025
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.
Commented-out retry constants suggest incomplete implementation. If retry logic is intentionally disabled, these constants should be removed; if retry functionality is still needed, the code should be uncommented and properly integrated.
| // const LAUNCH_RETRY_ATTEMPTS = 5; | |
| // const LAUNCH_RETRY_TIMEOUT = 20000; |
| // eslint-disable-next-line no-plusplus | ||
| testRunCounter++; | ||
| console.log(`🔄 Test run #${testRunCounter} started`); |
Copilot
AI
Oct 20, 2025
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.
Debug console.log statement should be removed or replaced with proper logging mechanism for production code.
| const executionCount = testExecutionMap.get(name); | ||
| if (executionCount! > 1) { | ||
| console.log(`🔁 Test "${name}" - retry attempt #${executionCount}`); | ||
| } else { | ||
| console.log(`✅ Test "${name}" - first run`); |
Copilot
AI
Oct 20, 2025
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.
Debug console.log statements should be removed or replaced with proper logging mechanism for production code.
| // await loadScripts(t, 'axe.min.js', { | ||
| // basePath: '../../../node_modules/axe-core/', | ||
| // checkExists: 'axe', | ||
| // }); |
Copilot
AI
Oct 20, 2025
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.
Commented-out code should be removed if no longer needed. If this is a temporary change or alternative implementation, add a comment explaining why it's preserved.
| // await loadScripts(t, 'axe.min.js', { | |
| // basePath: '../../../node_modules/axe-core/', | |
| // checkExists: 'axe', | |
| // }); |
e2e/testcafe-devextreme/runner.ts
Outdated
| } | ||
| process.exit(1); | ||
| }); | ||
| await testCafe.close(); |
Copilot
AI
Oct 20, 2025
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.
The test runner closes TestCafe immediately without waiting for test execution to complete. The runner.run() call and result handling logic appears to be missing, which will cause tests to never execute.
| // let testCafe: TestCafe; | ||
|
|
||
| // createTestCafe(TESTCAFE_CONFIG) | ||
| // .then(async (tc: TestCafe) => { | ||
| // testCafe = tc; | ||
|
|
||
| // const args = getArgs(); | ||
| // const testName = args.test.trim(); | ||
| // const reporter = typeof args.reporter === 'string' ? args.reporter.trim() : args.reporter; | ||
| // const indices = args.indices.trim(); | ||
| // let componentFolder = args.componentFolder.trim(); | ||
| // const file = args.file.trim(); | ||
|
|
||
| // setTestingPlatform(args); | ||
| // setTestingTheme(args); | ||
| // setShadowDom(args); | ||
|
|
||
| // componentFolder = componentFolder ? `${componentFolder}/**` : '**'; | ||
| // if (fs.existsSync('./screenshots')) { | ||
| // fs.rmSync('./screenshots', { recursive: true }); | ||
| // } | ||
|
|
||
| // const browsers = args.browsers |
Copilot
AI
Oct 20, 2025
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.
Large block of commented-out code (lines 352-560) should be removed. This appears to be the old implementation that has been replaced. If this code needs to be preserved for reference, it should be documented in version control history rather than kept as comments.
| // let testCafe: TestCafe; | |
| // createTestCafe(TESTCAFE_CONFIG) | |
| // .then(async (tc: TestCafe) => { | |
| // testCafe = tc; | |
| // const args = getArgs(); | |
| // const testName = args.test.trim(); | |
| // const reporter = typeof args.reporter === 'string' ? args.reporter.trim() : args.reporter; | |
| // const indices = args.indices.trim(); | |
| // let componentFolder = args.componentFolder.trim(); | |
| // const file = args.file.trim(); | |
| // setTestingPlatform(args); | |
| // setTestingTheme(args); | |
| // setShadowDom(args); | |
| // componentFolder = componentFolder ? `${componentFolder}/**` : '**'; | |
| // if (fs.existsSync('./screenshots')) { | |
| // fs.rmSync('./screenshots', { recursive: true }); | |
| // } | |
| // const browsers = args.browsers |
bfff62b to
d7ef866
Compare
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.
Pull Request Overview
Copilot reviewed 36 out of 36 changed files in this pull request and generated 5 comments.
| const pageHTML = await t.eval(() => document.documentElement.outerHTML); | ||
| console.log('=== FULL PAGE HTML ==='); | ||
| console.log(pageHTML); | ||
|
|
||
| const bodyHTML = await t.eval(() => document.body.innerHTML); | ||
| console.log('=== BODY HTML ==='); | ||
| console.log(bodyHTML); |
Copilot
AI
Oct 22, 2025
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.
Debug console.log statements should be removed before merging to production. These appear to be leftover debugging code from development.
| // const wait = async ( | ||
| // timeout: number, | ||
| // // eslint-disable-next-line no-promise-executor-return | ||
| // ): Promise<void> => new Promise((resolve) => setTimeout(resolve, timeout)); | ||
|
|
||
| // const retry = async <T>(action: () => Promise<T>, attempt: number): Promise<T> => { | ||
| // try { | ||
| // return await action(); | ||
| // } catch (error) { | ||
| // if (attempt <= 1) { | ||
| // throw error; | ||
| // } | ||
|
|
||
| // /* eslint-disable no-console */ | ||
| // console.log('\n > error occurred during testcafe launch!\n'); | ||
| // console.error(error); | ||
| // console.info(`\n > waiting ${LAUNCH_RETRY_TIMEOUT / 1000} seconds...\n`); | ||
| // await wait(LAUNCH_RETRY_TIMEOUT); | ||
| // console.info('\n > retry launching testcafe\n'); | ||
| // /* eslint-enable no-console */ | ||
| // return retry(action, attempt - 1); | ||
| // } | ||
| // }; |
Copilot
AI
Oct 22, 2025
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.
Large blocks of commented-out code reduce code readability and maintainability. If this retry mechanism is no longer needed, it should be removed entirely. If there's a possibility of needing it again, consider documenting the reason for its removal in a comment or tracking it in version control history instead.
| // let testCafe: TestCafe; | ||
|
|
||
| // createTestCafe(TESTCAFE_CONFIG) | ||
| // .then(async (tc: TestCafe) => { |
Copilot
AI
Oct 22, 2025
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.
Over 200 lines of commented-out code (lines 358-566) significantly reduce code readability. This appears to be the old implementation before refactoring to async/await. Since this code is preserved in version control, it should be removed entirely from the file.
| // interface LoadOptions { | ||
| // basePath?: string; | ||
| // checkExists?: string; | ||
| // } | ||
|
|
||
| // // async function loadResources( |
Copilot
AI
Oct 22, 2025
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.
Large blocks of commented-out code (lines 162-312) should be removed. This appears to be unused alternative implementations of resource loading. Since the code is tracked in version control, there's no need to keep it commented out in the source file.
| { componentFolder: "dataGrid/sticky/fixed", name: "dataGrid / sticky (3/3)", indices: "3/3" }, | ||
| { componentFolder: "cardView", name: "cardView" }, | ||
| { componentFolder: "cardView", name: "cardView - material", theme: 'material.blue.light' }, | ||
| { componentFolder: "cardView", name: "cardView - fluent", theme: 'fluent.blue.light' }, |
Copilot
AI
Oct 22, 2025
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.
The pivotGrid tests are commented out without explanation. Consider adding a comment explaining why these tests are disabled or create a tracking issue if this is temporary. YAML uses '#' for comments but mixing commented configuration with active configuration can lead to confusion.
| { componentFolder: "cardView", name: "cardView - fluent", theme: 'fluent.blue.light' }, | |
| { componentFolder: "cardView", name: "cardView - fluent", theme: 'fluent.blue.light' }, | |
| # The following pivotGrid tests are temporarily disabled due to known instability/issues. | |
| # See https://github.com/your-org/your-repo/issues/1234 for tracking and re-enabling. |
76cd081 to
021a22c
Compare
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.
Pull Request Overview
Copilot reviewed 34 out of 38 changed files in this pull request and generated 5 comments.
| const testExecutionMap = new Map<string, number>(); | ||
|
|
||
| const runOptions: RunOptions = { | ||
| quarantineMode: false, // { successThreshold: 1, attemptLimit: 1 }, |
Copilot
AI
Oct 24, 2025
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.
Quarantine mode is disabled with commented configuration inline. If this is intentional, the comment should be removed; otherwise, the proper configuration should be uncommented.
| quarantineMode: false, // { successThreshold: 1, attemptLimit: 1 }, | |
| quarantineMode: false, |
| if (!componentFolder.includes('accessibility')) { | ||
| // @ts-expect-error ts-errors | ||
| const { meta } = t.testRun.test; | ||
| console.log('Test meta:', meta); |
Copilot
AI
Oct 24, 2025
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.
Debug console logging should be removed before merging. This appears to be temporary debugging output.
| console.log('Test meta:', meta); |
| before: async () => { | ||
| // eslint-disable-next-line no-plusplus | ||
| testRunCounter++; | ||
| console.log(`🔄 Test run #${testRunCounter} started`); |
Copilot
AI
Oct 24, 2025
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.
Debug console logging should be removed or converted to a proper logger before merging. Multiple console.log statements for test execution tracking appear to be temporary debugging code.
|
|
||
| const executionCount = testExecutionMap.get(name); | ||
| if (executionCount! > 1) { | ||
| console.log(`🔁 Test "${name}" - retry attempt #${executionCount}`); |
Copilot
AI
Oct 24, 2025
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.
Debug console logging should be removed or converted to a proper logger before merging. Multiple console.log statements for test execution tracking appear to be temporary debugging code.
| if (executionCount! > 1) { | ||
| console.log(`🔁 Test "${name}" - retry attempt #${executionCount}`); | ||
| } else { | ||
| console.log(`✅ Test "${name}" - first run`); |
Copilot
AI
Oct 24, 2025
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.
Debug console logging should be removed or converted to a proper logger before merging. Multiple console.log statements for test execution tracking appear to be temporary debugging code.
No description provided.