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

Enhance toHaveValue's error message #218

Closed
maltebaer opened this issue Mar 19, 2020 · 6 comments
Closed

Enhance toHaveValue's error message #218

maltebaer opened this issue Mar 19, 2020 · 6 comments

Comments

@maltebaer
Copy link
Contributor

Describe the feature you'd like:

I often forget/do not consider the type I've set on my input while making assertions on it. When I mistakenly expect the wrong type using the toHaveValue matcher, my test will -- obviously -- fail with, e.g.

Expected the element to have value:
  8
Received:
  8

To already have an idea of what throws the error when looking at the console, the message could be enhanced with the values types

Expected the element to have value:
  8 (number)
Received:
  8 (string)

Suggested implementation:

When the expected and received value have loose equality, type information could be added to the output.

  let expectedTypedValue = expectedValue;
  let receivedTypedValue = receivedValue;
  if (expectedValue == receivedValue)  {
    expectedTypedValue = `${expectedValue} (${typeof expectedValue})`;
    receivedTypedValue = `${receivedValue} (${typeof receivedValue})`;
  }

Describe alternatives you've considered:

I did not consider any alternative solution since the expected changes are so small.

Teachability, Documentation, Adoption, Migration Strategy:

No need for the user to adopt the way the library is used.

As many developers suggest to get started with contributing to open-source by improving the tools/libraries you already use and if you consider this change a helpful improvement on the DX, I would be really happy to submit my first PR to an open-source project I discovered recently and already fell in love with.

Thank you for this great library ❤

@gnapse
Copy link
Member

gnapse commented Mar 19, 2020

This is a great suggestion indeed.

Would be great if you contributed the change as well. Are you up to?

The implementation is simple as you suggest. It's a matter of modifying this file, and in particular get the customized typed values into lines 38 and 40 in that fragment over there.

@maltebaer
Copy link
Contributor Author

maltebaer commented Mar 19, 2020

Awesome!

Let me watch http://kcd.im/pull-request and then I will contribute my solution 😃

function toHaveValue(htmlElement, expectedValue) {
  (0, _utils.checkHtmlElement)(htmlElement, toHaveValue, this);
  
  if (htmlElement.tagName.toLowerCase() === 'input' && ['checkbox', 'radio'].includes(htmlElement.type)) {
    throw new Error('input with type=checkbox or type=radio cannot be used with .toHaveValue(). Use .toBeChecked() for type=checkbox or .toHaveFormValues() instead');
  }
  
  const receivedValue = (0, _utils.getSingleElementValue)(htmlElement);
  const expectsValue = expectedValue !== undefined;

  let expectedTypedValue = expectedValue;
  let receivedTypedValue = receivedValue;
  if (expectedValue == receivedValue)  {
    expectedTypedValue = `${expectedValue} (${typeof expectedValue})`;
    receivedTypedValue = `${receivedValue} (${typeof receivedValue})`;
  }

  return {
    pass: expectsValue ? (0, _isEqualWith.default)(receivedValue, expectedValue, _utils.compareArraysAsSet) : Boolean(receivedValue),
    message: () => {
      const to = this.isNot ? 'not to' : 'to';
      const matcher = (0, _jestMatcherUtils.matcherHint)(`${this.isNot ? '.not' : ''}.toHaveValue`, 'element', expectedValue);
      return (0, _utils.getMessage)(matcher, `Expected the element ${to} have value`, expectsValue ? expectedTypedValue : '(any)', 'Received', receivedTypedValue);
    }
  };
}

@eps1lon
Copy link
Member

eps1lon commented Mar 19, 2020

Does toHaveValue check attributes or properties? If it checks attributes it should flat out throw a TypeError if it doesn't receive a string. For properties writing out the type would indeed be nice.

@gnapse
Copy link
Member

gnapse commented Mar 19, 2020

It checks the property. It even does some nice things to it (like gathering all selected option values from a multi-select into an array, or converting to number the value of a <input type="number" />). Check it out here.

@maltebaer
Copy link
Contributor Author

To me it looks like it checks the value property and not the attributes .

@afontcu
Copy link
Member

afontcu commented Apr 28, 2020

Solved in #219

@afontcu afontcu closed this as completed Apr 28, 2020
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

4 participants