Skip to content

Add checking end to end tests to PR review checklist #1317

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

Merged
merged 1 commit into from
Apr 8, 2025

Conversation

lfdebrux
Copy link
Member

@lfdebrux lfdebrux commented Apr 7, 2025

What problem does this pull request solve?

This PR should have the same changes as alphagov/forms-admin#1870.

We occasionally have cases where the end to end tests need to be changed to deploy changes because they rely on content in the app. It helps to think about this before merging a PR, so this commit adds a reminder to the PR review checklist to think about it.

Things to consider when reviewing

  • Ensure that you consider the wider context.
  • Does it work when run on your machine?
  • Is it clear what the code is doing?
  • Do the commit messages explain why the changes were made?
  • Are there all the unit tests needed?
  • Do the end to end tests need updating before these changes will pass?
  • Has all relevant documentation been updated?

We occasionally have cases where the end to end tests need to be
changed to deploy changes because they rely on content in the app. It
helps to think about this before merging a PR, so this commit adds a
reminder to the PR review checklist to think about it.
Copy link

sonarqubecloud bot commented Apr 7, 2025

@lfdebrux lfdebrux merged commit 884c516 into main Apr 8, 2025
5 checks passed
@lfdebrux lfdebrux deleted the ldeb-add-e2e-tests-to-review-checklist branch April 8, 2025 12:35
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