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

Hotfix and root cause analysis (RCA): Failing integration tests #113

Merged
merged 4 commits into from
Nov 2, 2022

Conversation

rezrah
Copy link
Collaborator

@rezrah rezrah commented Nov 1, 2022

Description

End-to-end integration tests for Astro and Next.js were - rather coincidentally - failing at the same time.

This was first detected in PR's, but hasn't impacted main branch yet as it hasn't run the integration tests since its last push.

This PR fixes the issue and serves as an RCA for what occurred, and how the problem was remediated.

Timeline*

Date Event
26 Oct 2022 Last commit to `main` branch
27 Oct 2022 Astro release v1.2.0, which includes a breaking change for non-tty users of its CLI package (create astro)
1 Nov 2022 (~1pm GMT) Integration test for Astro first observed to fail in a newly created pull request
1 Nov 2022 (~3pm GMT) Next.js release `v13.0.1` which includes new pre-build linting
1 Nov 2022 (~5pm GMT) Integration tests for Next.js first observed to fail in a newly created _and_ existing pull requests
2 Nov 2022 Remediation PR raised
  • using approximate times

Contributing Factors (Root Causes, Triggers)

  • Astro CLI stopped accepting non-tty standard input in its create astro CLI tooling, which Primer Brand relied on for test environment scaffolding. (Issue)
  • Next.js added next lint as a prebuild step in v13.0.1. This caused linting issues to fail the Next.js integration test
  • main branch didn't fail and notify maintainers because it hadn't run since the above packages were last updated.

Stabilization Steps

  • Manually debug steps to reproduce errors and remediate
  • Merge PR that verified fixes work, and push to main.

Lessons Learned

  • We needed main branch to notice these errors sooner. There was a gap of >4 days between release of the breaking change in Astro and when we first observed the failing build.
  • Our integration tests are great at catching build-time and runtime issues
  • We should expect UI frameworks to occasionally issue unintentional breaking changes as patch updates

Corrective Actions in this PR

  • This PR adds a daily cron schedule to all integration tests on the main branch.
  • Fix Astro by pegging release to v1.1, which still accepts non-tty stdin
  • Fix Next.js by a) prelinting and b) ignoring fixture related warnings

Next steps

  • Automatically create issues that notify maintainers on failing tests on main branch.

@changeset-bot
Copy link

changeset-bot bot commented Nov 1, 2022

⚠️ No Changeset found

Latest commit: 545680f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2022

🟢 No design token changes found

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2022

🟢 No visual differences found

Our visual comparison tests did not find any differences in the UI.

@rezrah rezrah temporarily deployed to github-pages November 1, 2022 17:11 Inactive
@rezrah rezrah temporarily deployed to github-pages November 1, 2022 17:32 Inactive
@rezrah rezrah changed the title Fixing integration tests Hotfix and root cause analysis (RCA): Failing integration tests Nov 2, 2022
@rezrah rezrah temporarily deployed to github-pages November 2, 2022 10:08 Inactive
@rezrah rezrah temporarily deployed to github-pages November 2, 2022 10:27 Inactive
@rezrah rezrah added the brand label Nov 2, 2022
@rezrah rezrah marked this pull request as ready for review November 2, 2022 10:46
@rezrah rezrah merged commit a3d14ef into main Nov 2, 2022
@rezrah rezrah deleted the hotfix/integration-tests-11-1-2022 branch November 2, 2022 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant