jest-native icon indicating copy to clipboard operation
jest-native copied to clipboard

toBeDisabled does not equal not.toBeEnabled

Open h-five-e opened this issue 3 years ago • 6 comments

  • react-native or expo: react-native
  • @testing-library/react-native version: 3.4.3
  • jest-preset: react-native
  • react-native version: 0.63.4
  • node version: v15.5.0

Relevant code or config:

it('toBeDisabled should equal not.toBeEnabled', async () => {
  const {getByTestId} = await render(
      <Pressable disabled={true}>
        <Pressable testID="subject" disabled={false} />
      </Pressable>,
  );
  const subject = getByTestId('subject');

  expect(subject).toBeDisabled();

  // This expectation fails
  expect(subject).not.toBeEnabled();
});

What you did:

Run this test.

What happened:

The second expectation fails, which I believe it should not do.

Reproduction:

I forked the template and made my changes (btw: the template is quite out of date, so I updated it to require the same versions as I run into this issue with). The relevant file is here: https://github.com/Punksmurf/ntl-sample/blob/master/src/tests/App-test.js

Problem description:

As a user, I expect toBeEnabled() and toBeDisabled() to be opposites, and therefore I expect .toBeDisabled() to yield the same result as .not.toBeEnabled(). This however is not the case.

I do not know if this is actually expected behaviour; the readme states that toBeEnabled() works similarly to .not.toBeDisabled(). It should also be noted that it indeed does; it's the other way around that I think is problematic.

The cause, as far as I can see, is in to-be-disabled.js:66. This differs from line 47 in that for the toBeDisabled() the ancestors are checked and for toBeEnabled() they are not.

Suggested solution:

Changing to-be-disabled.js:66 to var isEnabled = !(isElementDisabled(element) || isAncestorDisabled(element)); seems to fix the issue, however I am not comfortable recommending this change as I have almost no understanding of the internal workings of the testing library. Given that isAncestorDisabled() loops through quite a bit more ancestors (7) than I would expect (namely just the one), this is not just the view hierarchy and there may be subtleties at play that I do not know of.

Can you help us fix this issue by submitting a pull request?

I could create a PR with the above mentioned solution, if that is indeed the fix.

h-five-e avatar Mar 11 '21 11:03 h-five-e

Well that was quite an embarrassing mistake to make in the title.

h-five-e avatar Mar 20 '21 15:03 h-five-e

Just spent an hour or two trying to figure out why my element isn't coming up as disabled. Finally decided to place some logs in this library and found the exact same oddity as @Punksmurf . To be fank I don't think there is a right or wrong solution, there are cases when you want to check if a specific element is disabled (nested buttons) and cases where you want to check if the "whole branch" is disabled (non nested buttons). Same for enabled. BUT there is simply no reason for those two function to behave different from each other.

I assume the best solution would be two separate assertions, e.g. expect(e).toBeDisabledElement() for specific checks and expect(e).toBeDisabedBranch(), for, well, the whole branch. Yes, naming is hard. Maybe a flag passed to toBeDisabled/toBeEnabled could be the solution. E.g. function toBeDisabled(recursive?: boolean) [...]

For my (current) purposes I need the behavior of toBeDisabled and not toBeEnabled. I want to check a button to be enabled. I search for the text and the check will succeed even if the parent touchable (the actual button) has been disabled. I guess I will be using .not.toBeDisabled().

Elias-Graf avatar Apr 09 '21 14:04 Elias-Graf

Looking at the documentation, toBeDisabled (somewhat incorrectly) states that:

https://github.com/testing-library/jest-native/blob/3cfd279ad2a75a0da1baad8e366bf40c65b774dc/README.md#L119

It does in fact not just check if the parent has been disabled, but the whole branch, up to the tree root, right? (I've not looked at the code for too long so don't blame me too hard.). But hey minor detail.

On the other hand toBeEnable:

https://github.com/testing-library/jest-native/blob/3cfd279ad2a75a0da1baad8e366bf40c65b774dc/README.md#L147

This has the word similar in it, which means maybe this (rather important?) difference was supposed to be in the documentation?

But only documenting it is not enough (IMO), people are going to assume and not look at it until it's too late.

Elias-Graf avatar Apr 09 '21 15:04 Elias-Graf

I was playing with the test code above by swapping disabled. Not sure why both toBeDisabled and toBeEnabled still pass. Maybe the matcher for both cases also checks the parent but this is confusing.

it('toBeDisabled should equal not.toBeEnabled', async () => {
  const {getByTestId} = await render(
    <Pressable disabled={false}>
      <Pressable testID="subject" disabled={true} />
    </Pressable>,
  );
  const subject = getByTestId('subject');

  expect(subject).toBeDisabled();   // Pass
  expect(subject).toBeEnabled();    // Also pass

  // This expectation fails
  expect(subject).not.toBeEnabled(); // Fail
});

tomtanti avatar Aug 11 '21 05:08 tomtanti

Same issue ...

  it('Post button should be disabled there when is no image or text', async () => {
    const Element = <CreatePost />
    const { getByTestId } = render(Element)
    expect(getByTestId('postButton')).toBeDisabled() // pass
    expect(getByTestId('postButton')).toBeEnabled() // also pass
  })

matinzd avatar Nov 23 '21 23:11 matinzd

@tomtanti Did you find any solution? In my case, I have a custom buttom based on a TouchableOpacity and both of the assertions are passing. toBeDisabled and toBeEnabled both are passing even if the button is disabled

ibtisamarif831 avatar Jan 20 '22 11:01 ibtisamarif831

Fixed in #101

mdjastrzebski avatar Sep 30 '22 10:09 mdjastrzebski