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.

Sign up for GitHub

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

add job matrix #92

Open
wants to merge 10 commits into
base: pw
Choose a base branch
from
Open

add job matrix #92

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5290512
add job matrix
shashwatahalder01 Dec 12, 2024
d647885
fix yml lint
shashwatahalder01 Dec 12, 2024
5c1535c
Update reporter
shashwatahalder01 Dec 13, 2024
e6e478d
Fix yml lint
shashwatahalder01 Dec 13, 2024
2f2227f
Fix condition
shashwatahalder01 Dec 13, 2024
ab60fbf
test var
shashwatahalder01 Dec 13, 2024
d016d87
FIX ENV ISSUES
shashwatahalder01 Dec 13, 2024
6b6b825
Revert couple of changes
shashwatahalder01 Dec 13, 2024
8503d05
test lite pro parallel
shashwatahalder01 Dec 13, 2024
681612b
test parallel
shashwatahalder01 Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/e2e_api_tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ jobs:
tests:
name: e2e_api tests
runs-on: ubuntu-latest
strategy:
matrix:
test_group: [e2e_1, e2e_2]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix matrix strategy indentation.

The matrix strategy section has incorrect indentation. It should be indented under the strategy key.

Apply this fix:

        strategy:
-        matrix:
-            test_group: [e2e_1, e2e_2]
+          matrix:
+            group: [e2e_1, e2e_2]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
strategy:
matrix:
test_group: [e2e_1, e2e_2]
strategy:
matrix:
group: [e2e_1, e2e_2]
🧰 Tools
🪛 actionlint (1.7.4)

72-72: "strategy" section should not be empty. please remove this section if it's unnecessary

(syntax-check)

73-73: unexpected key "matrix" for "job" section. expected one of "concurrency", "container", "continue-on-error", "defaults", "env", "environment", "if", "name", "needs", "outputs", "permissions", "runs-on", "secrets", "services", "steps", "strategy", "timeout-minutes", "uses", "with"

(syntax-check)


steps:
- name: Checkout testing repo
Expand Down Expand Up @@ -161,13 +164,17 @@ jobs:
# npm run pw:deps-only

# Run e2e tests
- name: 🧪 Run e2e tests
- name: 🧪 Run e2e tests ${{ matrix.group }}
id: e2e-test
if: success() && (github.event_name != 'workflow_dispatch' || ( github.event_name == 'workflow_dispatch' && (github.event.inputs.testsuite == 'E2E' || github.event.inputs.testsuite == 'All')))
timeout-minutes: 40
working-directory: tests/pw
run: |
npm run test:e2e
if [[ "${{ matrix.group }}" == e2e_1 ]]; then
npx playwright test --project=${{ matrix.group }} --config=e2e.config.ts
elif [[ "${{ matrix.group }}" == a2e_2 ]]; then
npx playwright test --project=${{ matrix.group }} --config=e2e.config.ts
fi
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Simplify the test execution logic and fix the typo.

There are several issues in the test execution step:

  1. There's a typo in the elif condition: a2e_2 should be e2e_2
  2. The commands in both conditions are identical, making the if-elif structure unnecessary

Apply this fix:

-                if [[ "${{ matrix.group }}" == e2e_1 ]]; then
-                    npx playwright test --project=${{ matrix.group }} --config=e2e.config.ts
-                elif [[ "${{ matrix.group }}" == a2e_2 ]]; then
-                    npx playwright test --project=${{ matrix.group }} --config=e2e.config.ts
-                fi
+                npx playwright test --project=${{ matrix.group }} --config=e2e.config.ts
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [[ "${{ matrix.group }}" == e2e_1 ]]; then
npx playwright test --project=${{ matrix.group }} --config=e2e.config.ts
elif [[ "${{ matrix.group }}" == a2e_2 ]]; then
npx playwright test --project=${{ matrix.group }} --config=e2e.config.ts
fi
npx playwright test --project=${{ matrix.group }} --config=e2e.config.ts


# Run e2e coverage
- name: 🧪 Run e2e coverage
Expand Down
18 changes: 18 additions & 0 deletions tests/pw/e2e.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,24 @@ export default defineConfig({
// teardown: NO_SETUP ? undefined : 'coverage_report',
},


// e2e_tests
{
name: 'e2e_1',
testMatch: /.*\.spec\.ts/,
grep: [/@e2e_1/],
dependencies: NO_SETUP ? [] : ['e2e_setup'],
},
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

Remove or update the e2e_tests project to avoid test duplication

The codebase analysis confirms that there is an existing e2e_tests project that matches all .spec.ts files without any grep patterns, while the new e2e_1 and e2e_2 projects match the same files but with specific grep patterns. This creates an overlap where tests tagged with @e2e_1 and @e2e_2 will be executed twice - once by their respective projects and once by the e2e_tests project.

  • Remove the e2e_tests project if matrix execution with tags is the new intended way to run tests
  • Or add grep: [/^(?!.*@e2e_[12]).*$/] to the e2e_tests project to exclude tests tagged with @e2e_1 and @e2e_2
🔗 Analysis chain

Consider removing or updating the existing e2e_tests project.

The new matrix projects (e2e_1 and e2e_2) might overlap with the existing e2e_tests project since they all match the same test pattern. Consider either:

  1. Removing the e2e_tests project if it's no longer needed
  2. Adding a grep pattern to exclude @e2e_1 and @e2e_2 tagged tests

Let me check if there's an existing e2e_tests project configuration to verify the potential overlap.

Let me fix the regex and try again to find the e2e_tests configuration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if there are any test files that don't have matrix tags
rg -l "test\(" --type ts | xargs rg -L "@e2e_[12]"

Length of output: 299

Script:

#!/bin/bash
# Check for e2e_tests project configuration
rg -A 10 "e2e_tests.*{" tests/pw/e2e.config.ts

Length of output: 136

Script:

#!/bin/bash
# Check for e2e_tests project configuration
rg "name: ['\"](e2e_tests|e2e_1|e2e_2)['\"]" -B2 -A8 tests/pw/e2e.config.ts

Length of output: 942



// e2e_tests
{
name: 'e2e_2',
testMatch: /.*\.spec\.ts/,
grep: [/@e2e_2/],
dependencies: NO_SETUP ? [] : ['e2e_setup'],
},

// coverage_report
{
name: 'coverage_report',
Expand Down
4 changes: 2 additions & 2 deletions tests/pw/tests/e2e/admin.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ import { LoginPage } from '@pages/loginPage';
import { data } from '@utils/testData';

test.describe('Admin functionality test', () => {
test('admin can login', { tag: ['@lite', '@admin'] }, async ({ page }) => {
test('admin can login', { tag: ['@lite', '@admin', '@e2e_1'] }, async ({ page }) => {
const loginPage = new LoginPage(page);
await loginPage.adminLogin(data.admin);
});

test('admin can logout', { tag: ['@lite', '@admin'] }, async ({ page }) => {
test('admin can logout', { tag: ['@lite', '@admin', '@e2e_2'] }, async ({ page }) => {
const loginPage = new LoginPage(page);
await loginPage.adminLogin(data.admin);
await loginPage.logoutBackend();
Expand Down