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

toBeDisabled does not equal not.toBeEnabled #54

Closed
h-five-e opened this issue Mar 11, 2021 · 7 comments
Closed

toBeDisabled does not equal not.toBeEnabled #54

h-five-e opened this issue Mar 11, 2021 · 7 comments

Comments

@h-five-e
Copy link

  • 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 h-five-e changed the title toBeEnabled does not equal not.toBeEnabled toBeDisabled does not equal not.toBeEnabled Mar 20, 2021
@h-five-e
Copy link
Author

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

@Elias-Graf
Copy link

Elias-Graf commented Apr 9, 2021

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
Copy link

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

This matcher will check if the element or its parent has a `disabled` prop, or if it has

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:

Works similarly to `expect().not.toBeDisabled()`.

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.

@tomtanti
Copy link

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
});

@matinzd
Copy link

matinzd commented Nov 23, 2021

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
  })

@ibtisamarif831
Copy link

@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

@mdjastrzebski
Copy link
Collaborator

Fixed in #101

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

No branches or pull requests

6 participants