Skip to content

[Issue #8184] allow usage of playwright tags in e2e test jobs #8960

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

Merged
doug-s-nava merged 4 commits into from
Mar 13, 2026
Merged

[Issue #8184] allow usage of playwright tags in e2e test jobs #8960

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a08cc17
basic acceptance of tags in jobs
doug-s-nava Mar 10, 2026
9636b66
remove experimental action
doug-s-nava Mar 10, 2026
a097c75
updates from review
doug-s-nava Mar 12, 2026
9eed259
more review - use hardcoded url string
doug-s-nava Mar 12, 2026
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
22 changes: 20 additions & 2 deletions .github/actions/e2e/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ inputs:
description: "password for staging test user"
staging_test_user_mfa_key:
description: "mfa key for staging test user"
playwright_tags:
description: "pipe separated list of tags denoting groups of tests to run"
total_shards:
description: "total number of test shards in parent run"
default: "1"
Expand Down Expand Up @@ -77,7 +79,8 @@ runs:
npm install @playwright/test
npx playwright install --with-deps

- name: Run e2e tests (Shard ${{ inputs.shard }}/${{ inputs.total_shards }})
- name: Run all e2e tests (Shard ${{ inputs.current_shard }}/${{ inputs.total_shards }})
if: ${{ inputs.playwright_tags == '' }}
working-directory: ./frontend
env:
CI: true
Expand All @@ -88,10 +91,25 @@ runs:
STAGING_TEST_USER_PASSWORD: ${{ inputs.staging_test_user_password }}
STAGING_TEST_USER_MFA_KEY: ${{ inputs.staging_test_user_mfa_key }}
run: |
echo $STAGING_TEST_USER_EMAIL
npm run test:e2e
shell: bash


- name: Run e2e tests for ${{ inputs.playwright_tags }} (Shard ${{ inputs.current_shard }}/${{ inputs.total_shards }})
if: ${{ inputs.playwright_tags != '' }}
working-directory: ./frontend
env:
CI: true
TOTAL_SHARDS: ${{ inputs.total_shards }}
CURRENT_SHARD: ${{ inputs.current_shard }}
PLAYWRIGHT_TARGET_ENV: ${{ inputs.target }}
STAGING_TEST_USER_EMAIL: ${{ inputs.staging_test_user_email }}
STAGING_TEST_USER_PASSWORD: ${{ inputs.staging_test_user_password }}
STAGING_TEST_USER_MFA_KEY: ${{ inputs.staging_test_user_mfa_key }}
run: |
npm run test:e2e -- --grep "${{ inputs.playwright_tags }}"
shell: bash

- name: Debug logging on failure
if: ${{ failure() && inputs.api_logs == 'true' }}
working-directory: ./frontend
Expand Down
17 changes: 16 additions & 1 deletion .github/workflows/ci-frontend-e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,22 @@ on:
inputs:
api_logs:
description: "print api logs on failure?"
default: false
type: boolean
playwright_tags:
description: "pipe separated list of @tags denoting groups of tests to run"
required: false
type: string
workflow_call:
Copy link
Collaborator

@aoysimmons-GH aoysimmons-GH Mar 11, 2026

Choose a reason for hiding this comment

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

Maybe add these as well?

inputs:
  api_logs:
    description: "print api logs on failure?"
    default: "false"
    type: string
  playwright_tags:
    description: "pipe separated list of @tags denoting groups of tests to run"
    required: false
    type: string

inputs:
api_logs:
description: "print api logs on failure?"
default: false
type: boolean
playwright_tags:
description: "pipe separated list of @tags denoting groups of tests to run"
required: false
type: string
pull_request:
paths:
- frontend/**
Expand Down Expand Up @@ -99,7 +113,8 @@ jobs:
needs_node_setup: "false"
total_shards: ${{ matrix.total_shards }}
current_shard: ${{ matrix.shard }}
api_logs: ${{ inputs.api_logs }}
api_logs: ${{ inputs.api_logs && 'true' || 'false' }}
playwright_tags: ${{ inputs.playwright_tags }}
staging_test_user_email: ${{ secrets.STAGING_TEST_USER_EMAIL }}
staging_test_user_password: ${{ secrets.STAGING_TEST_USER_PASSWORD }}
staging_test_user_mfa_key: ${{ secrets.STAGING_TEST_USER_MFA_KEY }}
Expand Down
10 changes: 8 additions & 2 deletions .github/workflows/e2e-staging.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ on:
type: choice
options:
- staging
playwright_tags:
description: "pipe separated list of @tags denoting groups of tests to run"
required: false
type: string

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
Expand All @@ -35,14 +39,16 @@ jobs:
uses: actions/checkout@v6

- name: Set target
run: if [[ -z "${{ inputs.target }}" ]]; then echo "defaulted_target=staging" >> "$GITHUB_ENV"; else echo 'defaulted_target="${{ inputs.target }}"' >> "$GITHUB_ENV"; fi
run: if [[ -z "${{ inputs.target }}" ]]; then echo "defaulted_target=staging" >> "$GITHUB_ENV"; else echo "defaulted_target=${{ inputs.target }}" >> "$GITHUB_ENV"; fi

- uses: ./.github/actions/e2e
- name: Run E2E tests
uses: ./.github/actions/e2e
with:
version: ${{ inputs.version || github.ref }}
target: ${{ env.defaulted_target }}
total_shards: ${{ matrix.total_shards }}
current_shard: ${{ matrix.shard }}
playwright_tags: ${{ inputs.playwright_tags }}
staging_test_user_email: ${{ secrets.STAGING_TEST_USER_EMAIL }}
staging_test_user_password: ${{ secrets.STAGING_TEST_USER_PASSWORD }}
staging_test_user_mfa_key: ${{ secrets.STAGING_TEST_USER_MFA_KEY }}
Expand Down
31 changes: 16 additions & 15 deletions frontend/tests/e2e/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,24 @@ test.beforeEach(async ({ page }) => {
await page.goto("/", { waitUntil: "domcontentloaded", timeout });
});

test("has title", async ({ page }) => {
await expect(page).toHaveTitle(/Simpler.Grants.gov/);
test("has title", { tag: "@smoke" }, async ({ page }) => {
await expect(page).toHaveTitle(/Simpler\.Grants\.gov/);
});

test("clicking 'follow on GitHub' link opens a new tab pointed at Github repository", async ({
page,
context,
}) => {
const pagePromise = context.waitForEvent("page");
// Click the Follow on GitHub link
await page.getByRole("link", { name: "Follow on GitHub" }).click();
const newPage = await pagePromise;
await newPage.waitForLoadState();
await expect(newPage).toHaveURL(
/https:\/\/github.com\/HHS\/simpler-grants-gov/,
);
});
test(
Copy link
Collaborator

@aoysimmons-GH aoysimmons-GH Mar 11, 2026

Choose a reason for hiding this comment

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

I will do this:

test(
"clicking 'follow on GitHub' link opens a new tab pointed at Github repository",
{ tag: "@full-regression" },
async ({ page, context }) => {
const pagePromise = context.waitForEvent("page");
// Click the Follow on GitHub link
await page.getByRole("link", { name: "Follow on GitHub" }).click();
const newPage = await pagePromise;
await expect(newPage).toHaveURL(
/^https://github.com/HHS/simpler-grants-gov(?:/)?$/,
);
},
);

Copy link
Collaborator Author

@doug-s-nava doug-s-nava Mar 12, 2026

Choose a reason for hiding this comment

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

just to confirm, the change here is to change the regex from /https:\/\/github.com\/HHS\/simpler-grants-gov/ to /^https://github.com/HHS/simpler-grants-gov(?:/)?$/, or is there something else?

Copy link
Collaborator Author

@doug-s-nava doug-s-nava Mar 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We should escape the period, but with escaping, I think ^https:\/\/github\.com\/HHS\/simpler-grants-gov(?:\/)\?$ is what you're proposing, and I'm not sure how that is functionally any different from /https:\/\/github\.com\/HHS\/simpler-grants-gov in this case (where we're assuming that the protocol of the url will come at the beginning)

Copy link
Collaborator

@aoysimmons-GH aoysimmons-GH Mar 12, 2026

Choose a reason for hiding this comment

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

This is what I type but this git comment is not showing what I type in comment

Image

Copy link
Collaborator

@aoysimmons-GH aoysimmons-GH Mar 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
test(
test(
"clicking 'follow on GitHub' link opens a new tab pointed at Github repository",
{ tag: "@full-regression" },
async ({ page, context }) => {
const pagePromise = context.waitForEvent("page");
// Click the Follow on GitHub link
await page.getByRole("link", { name: "Follow on GitHub" }).click();
const newPage = await pagePromise;
await expect(newPage).toHaveURL(
/^https:\/\/github\.com\/HHS\/simpler-grants-gov(?:\/)?$/,
);
},
);

Copy link
Collaborator

@aoysimmons-GH aoysimmons-GH Mar 12, 2026

Choose a reason for hiding this comment

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

I just learned that I have to put the 'add a suggestion' to put code in comment. This is what I type.

"clicking 'follow on GitHub' link opens a new tab pointed at Github repository",
{ tag: "@full-regression" },
async ({ page, context }) => {
const pagePromise = context.waitForEvent("page");
// Click the Follow on GitHub link
await page.getByRole("link", { name: "Follow on GitHub" }).click();
const newPage = await pagePromise;
await newPage.waitForLoadState();
await expect(newPage).toHaveURL(
/^https:\/\/github\.com\/HHS\/simpler-grants-gov/,
Copy link
Collaborator

@aoysimmons-GH aoysimmons-GH Mar 12, 2026

Choose a reason for hiding this comment

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

This one will allow:
https://github.com/HHS/simpler-grants-gov/issues
https://github.com/HHS/simpler-grants-gov?x=1
Anything that starts with that prefix

This one only allows:

Suggested change
/^https:\/\/github\.com\/HHS\/simpler-grants-gov/,
/^https:\/\/github\.com\/HHS\/simpler-grants-gov(?:\/)?$/,

https://github.com/HHS/simpler-grants-gov
https://github.com/HHS/simpler-grants-gov/

Copy link
Collaborator Author

@doug-s-nava doug-s-nava Mar 12, 2026

Choose a reason for hiding this comment

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

got it - this does make this more specific. At this point it should probably just be a string though. I'll make that change

);
},
);

test("skips to main content when navigating via keyboard", async ({
page,
Expand Down
Loading