From b78bc530fdd571360b733043b8c317d6237ac500 Mon Sep 17 00:00:00 2001 From: "Aman Kumar [SSW]" <71385247+amankumarrr@users.noreply.github.com> Date: Tue, 2 Apr 2024 12:31:22 +1100 Subject: [PATCH] Fixing homepage broken image (#2363) * Fixing homepage broken image Affected Route: `/` Fixed : #2362 Adding device sizes array for SIZE props. https://nextjs.org/docs/pages/api-reference/components/image#devicesizes * Adding mobile browser for the Playwright * Adding browser for mobile safari * Modifying tests for the mobile screen * Running the template on pr trigger * Adding env for the template * Adding more attempts * Adding waituntil prop * Adding chromium browser for mobile viewport * Undo the changes in the template * Changing attempts to 2 from 4 * Updating the device name * Adding new pipeline to check images daily * Fixing syntax for the daily run * Adding checkout step * Adding a new step to extract env * echoing echo in a step * Adding shell to pwsh * Adding a new step to create issue * Adding checkout step * echoing test result * Adding guard to create an issue and removing custom reporter for the artifact * Extracting the output without reporter * Echoing test result in the logs * Changing the echo into write-ouput * Converting output into a string * Running test for the creating issue with right artifact * Fixing indentation in the issue template * Removing project and status from md * Removing custom reporter file * Removing trigger for PR and adding input for dispatch * Removing env step * Removing pull request trigger * Commenting out size prop * Update components/blocks/clientLogos.tsx Co-authored-by: Matt Wicks [SSW] * Adding a new step to add testpass value * Removing PR trigger * Adding create an issue in daily image test * Removing the invalid props for steps * Adding PR trigger for testing * Removing the create an issue to test the output variables * Adding the default value for the testpassed * Adding the chain condition within the block * Adding create issue step back in the flow * Matt's feedback * Removing the test pass step and related variables * Test run * Adding PR trigger for test * Adding typo for test * Adding seo-pr for test * Adding typo to test * Adding typo to test * Adding failure step to get the outcome * Passing props on failure * Adding continue on error * Adding testing output job * Changing the step name * removing single quote * Adding the step to output variable value * Changing the run value * Wrapping the value before publishing it * Adding output * echoing create issue value * Changing the value type * adding env to get the default value * Changing the condition * Adding true check for the job * removing double quote for true * adding output value for create issue * Adding new false for consistency * Adding extra condition * Wrapping with the variable name * removing condition * Adding extra echo * adding env for the github action * removing echo * adding quote to create issue variable * removing continue on true * Adding variable for the continue on error * failure condition for the job * Removing redundant steps * Updating the variables and cleanup * Removing error * removing pull request trigger * Adding step to checkout repo for the template --------- Co-authored-by: Matt Wicks [SSW] --- .github/ISSUE_TEMPLATE/auto_bug_report.md | 17 +++++ .github/workflows/daily-image-tests.yml | 70 +++++++++++++++++++++ .github/workflows/main-build-and-deploy.yml | 2 - .github/workflows/main-infra.yml | 1 - .github/workflows/pr-main-infra-check.yml | 1 - .github/workflows/pr-push-deploy.yml | 3 - .github/workflows/template-build.yml | 1 - .github/workflows/template-ui-tests.yml | 24 +++++++ components/blocks/clientLogos.tsx | 2 + next.config.mjs | 1 + playwright.config.ts | 12 ++-- ui-tests/components/bookingForm.spec.ts | 41 ------------ ui-tests/images.spec.ts | 4 +- 13 files changed, 122 insertions(+), 57 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/auto_bug_report.md create mode 100644 .github/workflows/daily-image-tests.yml delete mode 100644 ui-tests/components/bookingForm.spec.ts diff --git a/.github/ISSUE_TEMPLATE/auto_bug_report.md b/.github/ISSUE_TEMPLATE/auto_bug_report.md new file mode 100644 index 0000000000..2af8423014 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/auto_bug_report.md @@ -0,0 +1,17 @@ +--- +name: ๐Ÿ› Auto - Bug Report +about: Report a bug +title: "๐Ÿ› {{ env.TITLE }}" +labels: "Type: Bug" +assignees: "" +--- + +@SSWConsulting/ssw-website-maintainers + +**Describe the bug** +Pipeline has detected some broken images on the website. + +- [ ] Investigate and fix it asap. + +**Download the Playwright Artifact:** +https://github.com/SSWConsulting/SSW.Website/actions/runs/{{env.GITHUB_RUN_ID}}/artifacts/{{ env.ARTIFACT_ID}} for more details. diff --git a/.github/workflows/daily-image-tests.yml b/.github/workflows/daily-image-tests.yml new file mode 100644 index 0000000000..5b5cb15c7b --- /dev/null +++ b/.github/workflows/daily-image-tests.yml @@ -0,0 +1,70 @@ +name: Daily broken images check + +on: + schedule: + # Every Weekday at 8 AM AEDT - https://cron.help/#0_21_*_*_SUN-THU + - cron: "0 21 * * SUN-THU" + workflow_dispatch: + inputs: + deploy_url: + description: "The URL of the site to test" + required: true + tests_to_run: + description: "The tests to run" + required: true + create_issue: + description: "Create an issue if there are broken images" + type: boolean + default: true +defaults: + run: + shell: pwsh + +env: + create_issue: ${{ github.event.inputs.create_issue || true }} + +jobs: + check-broken-images: + name: Run Playwright Tests + uses: ./.github/workflows/template-ui-tests.yml + with: + deploy_url: ${{ inputs.deploy_url || 'https://ssw.com.au' }} + tests_to_run: ${{ inputs.tests_to_run || 'images'}} + continue-on-failure: true + + env-output: + name: Env Output + runs-on: ubuntu-latest + needs: check-broken-images + outputs: + create_issue: ${{ steps.envs.outputs.create_issue }} + steps: + - name: Env variables into Output # this is necessary as env variables are not accessible at Job level + id: envs + shell: pwsh + run: | + echo "Test passed : ${{ needs.check-broken-images.outputs.testPassed }}" + echo "Artifact Id: ${{ needs.check-broken-images.outputs.artifact-id }}" + $create_issue='${{ env.create_issue }}' + echo "create_issue=$create_issue" >> $env:GITHUB_OUTPUT + + create-an-issue: + name: GitHub Issue + runs-on: ubuntu-latest + needs: + - check-broken-images + - env-output + if: ${{ needs.env-output.outputs.create_issue == 'true' && needs.check-broken-images.outputs.testPassed == 'false'}} + steps: + - name: Checkout code + uses: actions/checkout@v2 + + - name: Create an Issue + uses: JasonEtco/create-an-issue@v2 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + TITLE: "Broken Images on Homepage" + ARTIFACT_ID: ${{ needs.check-broken-images.outputs.artifact-id }} + GITHUB_RUN_ID: ${{ github.run_id }} + with: + filename: .github/ISSUE_TEMPLATE/auto_bug_report.md diff --git a/.github/workflows/main-build-and-deploy.yml b/.github/workflows/main-build-and-deploy.yml index f8551bb8ed..47a4b4e669 100644 --- a/.github/workflows/main-build-and-deploy.yml +++ b/.github/workflows/main-build-and-deploy.yml @@ -73,7 +73,6 @@ jobs: uses: xom9ikk/dotenv@v2 with: path: ./.github - load-mode: strict - name: Deploy to staging id: deploy @@ -108,7 +107,6 @@ jobs: uses: xom9ikk/dotenv@v2 with: path: ./.github - load-mode: strict - name: Azure CLI - Login uses: azure/login@v2 diff --git a/.github/workflows/main-infra.yml b/.github/workflows/main-infra.yml index 4bfc287f03..27ad60c730 100644 --- a/.github/workflows/main-infra.yml +++ b/.github/workflows/main-infra.yml @@ -30,7 +30,6 @@ jobs: uses: xom9ikk/dotenv@v2 with: path: ./.github - load-mode: strict - name: Build run: | diff --git a/.github/workflows/pr-main-infra-check.yml b/.github/workflows/pr-main-infra-check.yml index bcb1aee236..e557f585b6 100644 --- a/.github/workflows/pr-main-infra-check.yml +++ b/.github/workflows/pr-main-infra-check.yml @@ -33,7 +33,6 @@ jobs: uses: xom9ikk/dotenv@v2 with: path: ./.github - load-mode: strict - name: Build run: | diff --git a/.github/workflows/pr-push-deploy.yml b/.github/workflows/pr-push-deploy.yml index 4f68b85f87..19717cd670 100644 --- a/.github/workflows/pr-push-deploy.yml +++ b/.github/workflows/pr-push-deploy.yml @@ -68,7 +68,6 @@ jobs: uses: xom9ikk/dotenv@v2 with: path: ./.github - load-mode: strict - name: Deploy to slot uses: ./.github/actions/deploy @@ -90,7 +89,6 @@ jobs: message: | Deployed changes to <${{ steps.deploy.outputs.url }}> repo-token: ${{ secrets.GITHUB_TOKEN }} - repo-token-user-login: "github-actions[bot]" allow-repeats: true ui-test: @@ -137,5 +135,4 @@ jobs: View the full report repo-token: ${{ secrets.GITHUB_TOKEN }} - repo-token-user-login: "github-actions[bot]" allow-repeats: true diff --git a/.github/workflows/template-build.yml b/.github/workflows/template-build.yml index 5a67af98ba..54d40fc371 100644 --- a/.github/workflows/template-build.yml +++ b/.github/workflows/template-build.yml @@ -40,7 +40,6 @@ jobs: uses: xom9ikk/dotenv@v2 with: path: ./.github - load-mode: strict - name: Get current date id: date diff --git a/.github/workflows/template-ui-tests.yml b/.github/workflows/template-ui-tests.yml index c9091e1186..44c9ff78ec 100644 --- a/.github/workflows/template-ui-tests.yml +++ b/.github/workflows/template-ui-tests.yml @@ -9,11 +9,25 @@ on: tests_to_run: type: string required: true + continue-on-failure: + type: boolean + required: false + outputs: + artifact-id: + description: "Playwright Artifact ID" + value: ${{ jobs.test.outputs.artifact-id }} + testPassed: + description: "Test Passed" + value: ${{ jobs.test.outputs.testPassed }} jobs: test: name: Run Playwright tests runs-on: ubuntu-latest + continue-on-error: ${{ inputs.continue-on-failure || false }} + outputs: + testPassed: ${{ steps.onFailure.outputs.testPassed || 'true' }} + artifact-id: ${{ steps.artifact-report.outputs.artifact-id }} steps: - uses: actions/checkout@v4 @@ -42,12 +56,22 @@ jobs: retry-delay: 10s - name: Run Playwright tests + id: run run: npx playwright test ${{ inputs.tests_to_run }} env: HOST_URL: ${{ inputs.deploy_url }} + - name: Run on failure + id: onFailure + shell: pwsh + if: failure() && steps.run.outcome == 'failure' + run: | + $testPassed='false' + echo "testPassed=$testPassed" >> $env:GITHUB_OUTPUT + - uses: actions/upload-artifact@v4 if: always() + id: artifact-report with: name: playwright-report path: playwright-report/ diff --git a/components/blocks/clientLogos.tsx b/components/blocks/clientLogos.tsx index b3364f71a3..ef9f3600b2 100644 --- a/components/blocks/clientLogos.tsx +++ b/components/blocks/clientLogos.tsx @@ -15,6 +15,8 @@ export const ClientLogos = () => { alt={client.clientName + " logo"} height={113} width={200} + // commented out to test whether this is breaking images on the homepage see #2368 + //sizes="20vw" className="my-4 max-w-full rounded-lg" /> ))} diff --git a/next.config.mjs b/next.config.mjs index 31c1a824b2..2ab27d873a 100644 --- a/next.config.mjs +++ b/next.config.mjs @@ -8,6 +8,7 @@ const config = { }, poweredByHeader: false, images: { + deviceSizes: [384, 640, 750, 828, 1080, 1200, 1920, 2048, 3840], remotePatterns: [ { protocol: "https", diff --git a/playwright.config.ts b/playwright.config.ts index abce3af4d8..1b1c89e84d 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -50,13 +50,13 @@ export default defineConfig({ // }, /* Test against mobile viewports. */ + { + name: "Mobile Chrome", + use: { ...devices["Pixel 5"] }, + }, // { - // name: 'Mobile Chrome', - // use: { ...devices['Pixel 5'] }, - // }, - // { - // name: 'Mobile Safari', - // use: { ...devices['iPhone 12'] }, + // name: "Mobile Safari", + // use: { ...devices["iPhone 12"] }, // }, /* Test against branded browsers. */ diff --git a/ui-tests/components/bookingForm.spec.ts b/ui-tests/components/bookingForm.spec.ts deleted file mode 100644 index c8a65e39b7..0000000000 --- a/ui-tests/components/bookingForm.spec.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { expect, test } from "@playwright/test"; - -test("Can submit the booking form", async ({ page }) => { - await page.goto("/consulting/angular", { waitUntil: "networkidle" }); - - await page - .getByRole("button") - .getByText("Book a FREE Initial Meeting") - .first() - .click(); - - await page.waitForSelector(".react-responsive-modal-root form"); - - // note: preference would be to reference by label, but the names aren't consistent with the field names - // note: getByPlaceholder is doing a partial match (the asterisks are hardcoded into the placeholder text) ๐Ÿคฎ - await page.getByPlaceholder("your full name").fill("๐Ÿงช Test"); - await page.getByPlaceholder("your email").fill("amankumar@ssw.com.au"); - await page.getByPlaceholder("your phone").fill("0000000000"); - await page.locator("select[name='location']").selectOption("Australia"); // placeholder looks like location, but its actually an unselectable option - await page.locator("select[name='states']").selectOption("100000001"); - await page.getByPlaceholder("your company").fill("Test"); - await page.locator("select[name='referralSource']").selectOption("14"); - await page - .getByPlaceholder("how can we help you") - .fill( - `

Hi Account Managers,

This is a weekly test email to verify that the create lead flow is working. ${ - process.env.RECAPTCHA_BYPASS_SECRET ?? "No Key found" - }` - ); - - await page.locator("button[type='submit']").first().click(); - - const successToastId = "#success-toaster"; - await page.waitForSelector(successToastId); - const toastElement = await page.locator(successToastId).first(); - const toastText = await toastElement.textContent(); - - await expect(toastText).toBe( - "Form submitted. We'll be in contact as soon as possible." - ); -}); diff --git a/ui-tests/images.spec.ts b/ui-tests/images.spec.ts index 0ce5136c1d..e149b1048e 100644 --- a/ui-tests/images.spec.ts +++ b/ui-tests/images.spec.ts @@ -7,7 +7,7 @@ test("Images load successfully on index", async ({ page }) => { } }); - const response = await page.goto("/", { waitUntil: "networkidle" }); + const response = await page.goto("/", { waitUntil: "commit" }); expect(response.ok()).toBeTruthy(); // Taken from https://github.com/microsoft/playwright/issues/19277 @@ -32,7 +32,7 @@ test("Images load successfully on consulting page", async ({ page }) => { }); const response = await page.goto("/consulting/react", { - waitUntil: "networkidle", + waitUntil: "commit", }); expect(response.ok()).toBeTruthy();