Enforce consistent assertions

[skovy]

The prefer-explicit-assert rule enforces an explicit assertion but allows a large number of possible assertions. For example:

expect(getByText('foo')).toBeDefined();
expect(getByText('foo')).toBeTruthy();
expect(getByText('foo')).not.toBeNull();
expect(getByText('foo')).toBeInTheDocument();

Functionally, these are all the same (since getBy* throws) but in a large codebase, it's possible these can all be used interchangeably. It would be nice to enforce a consistent assertion on getBy* queries. Even though these behave the same with getBy* (since it throws or always returns something) it's misleading mixing them since toBeDefined is explicitly only checking that it's not undefined, but toBeTruthy is checking that it's not six values: false, 0, '', null, undefined, and NaN.

One way this could be added is by adding a new configuration option:

"testing-library/prefer-explicit-assert": ["error", {"assertion": "toBeInTheDocument"}],

Or, it could be added as a new rule (eg: prefer-consistent-assert).

Happy to put together a PR that makes these changes, but wanted to get feedback if this aligns with the intent of these linting rules and also the preferred implementation?

[Belco90]

Hello Spencer! Thanks for your suggestions, this is a really good idea actually.

When I originally implemented prefer-explicit-assert I wanted to allow different assertions checking truthy. You made a good point about adding the possibility to customize which of those assertions is considered a valid explicit one, which ideally always should be toBeInTheDocument.

I'm happy to add config option to the rule as you described, I think it's simple and makes sense to be part of this rule.

[Belco90]

🎉 This issue has been resolved in version 3.7.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[skovy]

@Belco90 thanks for the quick feedback and review!

I'm now realizing I entirely forgot about a valid set of use cases to not use toBeInTheDocument, for example:

expect(screen.getByText("foo")).toBeDisabled();
expect(screen.getByText("foo")).toHaveAttribute("href", "/foo");
// etc.

The quickest solution I can think of is to create a list of "truthy" or "existence" matchers (toBeDefined, toBeTruthy, not.toBeNull) and only lint if one of those is used instead of the defined assertion.

Do you have any other thoughts on how to handle/ignore these valid use cases?

[Belco90]

I'm not sure if I'm following. You mean to consider .not.toBeInTheDocument valid too?

[skovy]

Sorry, I didn't explain it very well. With the way this was implemented, any getBy* query will trigger a linting error if it's not using toBeInTheDocument (assuming the config defined the assertion to use as toBeInTheDocument). However, there are still valid cases to use a different assertion. For example, if you want to check if a button is disabled (toBeDisabled) or maybe check if an anchor has the appropriate href attribute (toHaveAttribute).

With the way the rule is currently implemented, both of these examples will be triggered as violations (since they're not using toBeInTheDocument) but they're both still valid uses of getBy*. This rule change doesn't seem to be very useful since it's triggering false positives.

I was trying to think of solutions to this problem, or see if you have any ideas?

The only idea I currently have is to check if the matcher is toBeDefined, toBeTruthy, toBeInTheDocument, or not.toBeNull and only trigger the error if an expectation is using one of these matchers and it doesn't match the defined assertion. This could be fragile if a new "truthy" matcher is added or missed (eg: toExist or something), but I can't think of any other way to avoid these false positives at the moment.

[Belco90]

Ok got it now. Didn't think about this either! I think just check configured valid assertion through options against the current list of "truthy" matchers is the best option. Everything else seems too brittle.

I think with this approach, the rule should throw an error if config assertion is not one of the options from that truthy list. Otherwise users could opt-out this rule without knowing.

[github-actions]

🎉 This issue has been resolved in version 4.0.0-beta.1 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀