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

Improve test diagnostics #504

Merged
merged 8 commits into from
Jun 4, 2024
Merged

Improve test diagnostics #504

merged 8 commits into from
Jun 4, 2024

Conversation

textbook
Copy link
Member

@textbook textbook commented Jun 2, 2024

The failing states of the tests weren't particularly useful, so:

  • expect(foo.length).toBe(bar) -> expect(foo).toHaveLength(bar), because the latter shows you what was in foo if its length isn't bar - see e.g. jest/prefer-to-have-length
  • expect(foo[0].bar).toBe(baz) -> expect(foo).toEqual([{ bar: baz }]) again because then the failing test tells you what foo was, rather than "expected baz received undefined" (or crashing with TypeError: Cannot read properties of undefined)
    • Another alternative, which may be useful in some circumstances, would be expect(foo).toHaveProperty([0, "bar"], baz)

From Growing Object-Oriented Software by Nat Pryce and Steve Freeman, © 2010 Nat Pryce, released under CC BY-SA 4.0.

Also:

  • \c videorec; broke the test seeding
  • Depending on the implementation, await waitForElementToBeRemoved(deleteButton); can fail because the same button is reused by React - wait for there to be only one button instead
  • Use role-based selectors to give a11y feedback
  • Use user-event interactions to let the tests focus on behaviour
  • An idiomatic delete is 204 No Content, as the resource no longer exists to return

@textbook textbook requested review from sztupy and Dedekind561 June 2, 2024 10:22
Copy link
Contributor

@Dedekind561 Dedekind561 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @textbook!

@Dedekind561 Dedekind561 merged commit e443210 into main Jun 4, 2024
3 checks passed
@Dedekind561 Dedekind561 deleted the chore/test-diagnostics branch June 4, 2024 10:50
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