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

Refactor the list collector repeating blocks functional test spec #1240

Merged
merged 4 commits into from
Nov 7, 2023

Conversation

berroar
Copy link
Contributor

@berroar berroar commented Oct 30, 2023

What is the context of this PR?

Refactored the tests in the list collector repeating blocks spec file. Needed to be refactored as some of the test cases were missing assertions or were executing additional steps after assertions. All tests should now be following the Given, When, Then pattern.

How to review

Check that the changes make sense and that the functional test still passes. Are any test cases missing?

Checklist

  • New static content marked up for translation
  • Newly defined schema content included in eq-translations repo

@berroar
Copy link
Contributor Author

berroar commented Oct 30, 2023

Copy link
Contributor

@katie-gardner katie-gardner left a comment

Choose a reason for hiding this comment

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

This is way better, nice 👍
Changes look good, includes all the correct tests with proper assertions
Only minor thought was that the describe headers were slightly inconsistent, some quite detailed and some not, reckon middle ground for those would be nice

Copy link
Contributor

@Yuyuutsu Yuyuutsu left a comment

Choose a reason for hiding this comment

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

Changes look good and working as expected 👍

@berroar berroar merged commit 131c18e into main Nov 7, 2023
15 checks passed
@berroar berroar deleted the refactor-list-collector-repeating-blocks-spec branch November 7, 2023 09:45
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.

4 participants