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 page load time testing #653

Merged
merged 6 commits into from
Jan 19, 2024
Merged

Add page load time testing #653

merged 6 commits into from
Jan 19, 2024

Conversation

greg-does-weather
Copy link
Collaborator

@greg-does-weather greg-does-weather commented Jan 17, 2024

What does this PR do? 🛠️

Heavily inspired by #576

  • Adds page load time testing for five locations. Asserts a maximum single page load time of 5 seconds and a maximum average of 3 seconds. These seem like reasonable points to throw big alarms like test failures.
    • I don't think we should make PRs dependent on these timing tests passing. They're too outside of our control. But we still need to see it, and it seems like a big ugly red icon on our PRs is a good way to make sure it's in our faces.
  • Tweaks the code standards workflow
    • moves the conditionals out of the various job steps and into the jobs themselves so there's just less duplication
    • reworks the Docker build caching; the old way just didn't work
      • builds our Drupal image into a tar file and caches that
      • about 30 seconds slower when the image needs to be rebuilt, but about a minute faster the rest of the time

What does the reviewer need to know? 🤔

nothing special

@greg-does-weather greg-does-weather marked this pull request as draft January 17, 2024 14:39
@greg-does-weather greg-does-weather changed the title load time testing Add page load time testing Jan 17, 2024
@greg-does-weather greg-does-weather marked this pull request as ready for review January 17, 2024 15:23
Copy link
Collaborator

@eric-gade eric-gade left a comment

Choose a reason for hiding this comment

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

Looks good, makes sense, works locally. I pushed one commit to add a lt directive to the Makefile for quickly running these tests.

Code Review Checklist

The Basics

  • Checks are passing
  • I read the code
  • I ran the code
  • (if applicable) Post deploy steps are run
  • (if applicable) I validated the change on the deployed version in

Documentation

  • changes to “how we do things” are documented in READMEs
  • all new functions and methods are commented using plain language
  • any new modules added documented in modules.md

Security

  • security false positives are documented
  • data from external sources is cleaned and clearly marked

Reliability

  • error handling exists for unusual or missing values
  • interactions with external systems are wrapped in try/except
  • functionality is tested with unit or integration tests
  • dependency updates in composer.json also got changed in composer-lock.json

Infrastructure

  • all changes are auditable and documented via a script
  • it is clear who can and should run the script
  • (if applicable) diagrams have been updated or added in PlantUML

Accessibility

  • New pages have been added to cypress-axe file so that they will be tested with our automated accessibility testing
  • Meets WCAG 2.0 AA or 2.1 AA for Section 508 compliance
    • Site is keyboard accessible. All interactions can be accessed with a keyboard
    • Site is free of keyboard traps. The keyboard focus is never trapped in a loop
    • All form inputs have explicit labels
    • Form instructions are associated with inputs
    • All relevant images use an img tag
    • All images have appropriate alt attributes
    • Multimedia is tagged. All multimedia has appropriate captioning and audio description
    • Text has sufficient color contrast. All text has a contrast ratio of 4.5:1 with the background
    • Site never loses focus. Focus is always visible when moving through the page with the keyboard
    • Tab order is logical
    • Tables are coded properly. Tables have proper headers and column attributes
    • Headings are nested properly. Heading elements are nested in a logical way
    • Language is set. The language for the page is set
    • CSS is not required to use the page. The page makes sense with or without CSS
    • Links are unique and contextual. All links can be understood taken alone, e.g., ‘Read more - about 508’
    • Page titles are descriptive

Device Matrix

  • firefox/gecko (renders correctly and user interactions work)
  • chrome/chromium/edge (renders correctly and user interactions work)
  • safari/webkit (renders correctly and user interactions work)
  • web page is readable and usable
    • at 480px (mobile)
    • at 640px (tablet)
    • at 1024px (desktop)

@greg-does-weather
Copy link
Collaborator Author

greg-does-weather commented Jan 19, 2024

Oh bless it... trying to re-sign the commit and I accidentally dropped the Makefile change... 🤦‍♂️ nm, it's still there I guess

@greg-does-weather greg-does-weather added this pull request to the merge queue Jan 19, 2024
Merged via the queue into main with commit c13a2e0 Jan 19, 2024
10 checks passed
@greg-does-weather greg-does-weather deleted the mgwalker/load-times branch January 19, 2024 17:07
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants