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

add job matrix #92

Open
wants to merge 10 commits into
base: pw
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
64 changes: 39 additions & 25 deletions .github/workflows/e2e_api_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,15 @@ env:
SHA: ${{ github.event.pull_request.head.sha }}
SYSTEM_INFO: ./tests/pw/playwright/systemInfo.json
API_TEST_RESULT: ./tests/pw/playwright-report/api/summary-report/results.json
E2E_TEST_RESULT: ./tests/pw/playwright-report/e2e/summary-report/results.json
API_COVERAGE: ./tests/pw/playwright-report/api/coverage-report/coverage.json
E2E_COVERAGE: ./tests/pw/playwright-report/e2e/coverage-report/coverage.json

jobs:
tests:
name: e2e_api tests
name: e2e_api tests-${{ matrix.group }}
runs-on: ubuntu-latest
strategy:
matrix:
group: [e2e_1, e2e_2]

steps:
- name: Checkout testing repo
Expand Down Expand Up @@ -115,17 +116,17 @@ jobs:
cd tests/pw
npm run start:env

# Set permalink structure
- name: Set Permalink structure
working-directory: tests/pw
run: |
npm run wp-env run tests-cli wp rewrite structure /%postname%/
# # Set permalink structure
# - name: Set Permalink structure
# working-directory: tests/pw
# run: |
# npm run wp-env run tests-cli wp rewrite structure /%postname%/

# Activate theme
- name: Activate theme:Storefront
working-directory: tests/pw
run: |
npm run wp-env run tests-cli wp theme activate storefront
# # Activate theme
# - name: Activate theme:Storefront
# working-directory: tests/pw
# run: |
# npm run wp-env run tests-cli wp theme activate storefront
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Re-enable commented out steps - they are critical dependencies

The verification shows these steps are essential and actively used in the test suite:

  • Permalink structure setup is required for product permalink tests in productsDetails.spec.ts and site setup in _site.setup.ts
  • Storefront theme is a critical dependency used across multiple setup files (_site.setup.ts, _localSite.setup.ts, _env.setup.ts) including theme-specific configurations
  • API tests are configured in package.json and have dedicated test commands

Locations requiring attention:

  • .github/workflows/e2e_api_tests.yml: Re-enable lines 121-131 (permalink and theme setup)
  • .github/workflows/e2e_api_tests.yml: Re-enable lines 187-193 (API tests execution)
🔗 Analysis chain

Verify impact of disabled steps

Several critical steps have been commented out:

  1. Permalink structure setup
  2. Theme activation
  3. API tests execution

These changes could impact the test environment setup and overall test coverage.

Let's verify if these steps are still needed:

Also applies to: 187-193


Let me check the actual content of these files to understand the dependencies better.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if permalink structure is referenced in tests
rg -l "permalink" tests/pw/tests/e2e/

# Check if theme-dependent selectors are used in tests
rg -l "storefront" tests/pw/tests/e2e/

# Check for API test dependencies
rg -l "test:api" tests/pw/package.json

Length of output: 320


Script:

#!/bin/bash
# Check permalink usage in test files
rg "permalink" tests/pw/tests/e2e/productsDetails.spec.ts tests/pw/tests/e2e/_site.setup.ts -C 2

# Check storefront theme dependencies
rg "storefront" tests/pw/tests/e2e/_site.setup.ts tests/pw/tests/e2e/_localSite.setup.ts tests/pw/tests/e2e/_env.setup.ts -C 2

# Check API test configuration
rg "test:api" tests/pw/package.json -C 2

Length of output: 3065


# Get Playwright version
- name: Get installed Playwright version
Expand Down Expand Up @@ -161,29 +162,42 @@ 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
timeout-minutes: 30
working-directory: tests/pw
run: |
npm run test:e2e
if [[ "${{ matrix.group }}" == "e2e_1" ]]; then
export E2E_TEST_RESULT="./tests/pw/playwright-report/e2e/${{ matrix.group }}/summary-report/results.json"
export E2E_COVERAGE="./tests/pw/playwright-report/e2e/${{ matrix.group }}/coverage-report/coverage.json"
export GROUP="${{ matrix.group }}"
echo "E2E_TEST_RESULT: $E2E_TEST_RESULT"
echo "E2E_COVERAGE: $E2E_COVERAGE"
echo "GROUP: $GROUP"
npx playwright test --project="${{ matrix.group }}" --config=e2e.config.ts
elif [[ "${{ matrix.group }}" == "e2e_2" ]]; then
export E2E_TEST_RESULT="./tests/pw/playwright-report/e2e/${{ matrix.group }}/summary-report/results.json"
export E2E_COVERAGE="./tests/pw/playwright-report/e2e/${{ matrix.group }}/coverage-report/coverage.json"
export GROUP="${{ matrix.group }}"
npx playwright test --project="${{ matrix.group }}" --config=e2e.config.ts
fi

# Run e2e coverage
- name: 🧪 Run e2e coverage
if: always() && (steps.e2e-test.outcome == 'success' || steps.e2e-test.outcome == 'failure')
working-directory: tests/pw
run: |
npm run test:e2e:coverage
npm run test:e2e:coverage

# Run api tests
- name: 🧪 Run api tests
id: api-test
if: always() && steps.wp-env.outcome == 'success' && ( github.event_name != 'workflow_dispatch' || ( github.event_name == 'workflow_dispatch' && (github.event.inputs.testsuite == 'API' || github.event.inputs.testsuite == 'All')))
timeout-minutes: 5
working-directory: tests/pw
run: |
npm run test:api
# - name: 🧪 Run api tests
# id: api-test
# if: always() && steps.wp-env.outcome == 'success' && ( github.event_name != 'workflow_dispatch' || ( github.event_name == 'workflow_dispatch' && (github.event.inputs.testsuite == 'API' || github.event.inputs.testsuite == 'All')))
# timeout-minutes: 5
# working-directory: tests/pw
# run: |
# npm run test:api
Comment on lines +187 to +193
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Re-enable API test execution.

The API tests have been commented out without justification. These tests are configured in package.json and are an essential part of the test suite.

Remove the comment markers to restore API test coverage.


# Prepare test summary
- name: Prepare test summary
Expand Down Expand Up @@ -229,7 +243,7 @@ jobs:
uses: actions/upload-artifact@v4
if: always() && steps.debug-log.outcome == 'success'
with:
name: test-artifact
name: test-artifact-${{ matrix.group }}
path: |
tests/pw/wp-data
tests/pw/playwright
Expand Down
2 changes: 1 addition & 1 deletion tests/pw/api.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export default defineConfig({
/* Configure reporters */
reporter: CI
? [
['github'],
// ['github'],
['html', { open: 'never', outputFolder: 'playwright-report/api/html-report' }],
// ['junit', { outputFile: 'playwright-report/api/junit-report/api-results.xml' }],
['list', { printSteps: true }],
Expand Down
20 changes: 18 additions & 2 deletions tests/pw/e2e.config.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { defineConfig, devices, expect } from '@playwright/test';
import { customExpect } from '@utils/pwMatchers';
import 'dotenv/config';
const { CI, NON_HEADLESS, BASE_URL, SLOWMO, NO_SETUP, DOKAN_PRO } = process.env;
const { CI, NON_HEADLESS, BASE_URL, SLOWMO, NO_SETUP, DOKAN_PRO, GROUP } = process.env;

export default defineConfig({
/* test directory */
Expand Down Expand Up @@ -59,7 +59,7 @@ export default defineConfig({
['html', { open: 'never', outputFolder: 'playwright-report/e2e/html-report' }],
// ['junit', { outputFile: 'playwright-report/e2e/junit-report/e2e-results.xml' }],
['list', { printSteps: true }],
['./utils/summaryReporter.ts', { outputFile: 'playwright-report/e2e/summary-report/results.json' }],
['./utils/summaryReporter.ts', { outputFile: `playwright-report/e2e/${GROUP}/summary-report/results.json` }],
],

use: {
Expand Down Expand Up @@ -151,6 +151,22 @@ 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'],
},

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

// coverage_report
{
name: 'coverage_report',
Expand Down
6 changes: 3 additions & 3 deletions tests/pw/tests/e2e/_coverage.teardown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fs from 'fs';
import path from 'path';
import { helpers } from '@utils/helpers';

const { DOKAN_PRO } = process.env;
const { DOKAN_PRO, GROUP } = process.env;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for required GROUP environment variable

The GROUP variable is destructured but not validated. Consider adding a validation check to ensure it's defined.

-const { DOKAN_PRO, GROUP } = process.env;
+const { DOKAN_PRO, GROUP } = process.env;
+if (!GROUP) {
+  throw new Error('GROUP environment variable is required for coverage reporting');
+}
📝 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
const { DOKAN_PRO, GROUP } = process.env;
const { DOKAN_PRO, GROUP } = process.env;
if (!GROUP) {
throw new Error('GROUP environment variable is required for coverage reporting');
}


let executed_tests: string[] = [];

Expand All @@ -17,8 +17,8 @@ const uncoveredFeatures: string[] = [];

teardown.describe('get e2e test coverage', () => {
const feature_map = 'feature-map/feature-map.yml';
const outputFile = 'playwright-report/e2e/coverage-report/coverage.json';
const testReport = 'playwright-report/e2e/summary-report/results.json';
const outputFile = `playwright-report/e2e/${GROUP}/coverage-report/coverage.json`;
const testReport = `playwright-report/e2e/${GROUP}/summary-report/results.json`;

teardown('get coverage', { tag: ['@lite'] }, async () => {
executed_tests = helpers.readJson(testReport)?.tests;
Expand Down
4 changes: 2 additions & 2 deletions tests/pw/tests/e2e/admin.spec.ts
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
1 change: 1 addition & 0 deletions tests/pw/types/environment.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ declare global {
E2E_TEST_RESULT: string;
API_COVERAGE: string;
E2E_COVERAGE: string;
GROUP: string;
}
}
}
Loading