toHaveStyle() test "Uses px as the default unit" always yields positive result + isSubset() is leaky

[revelt]

• @testing-library/jest-dom version: current master branch, I guess 5.14.1
• node version: v12.13.1
• npm (or yarn) version: 6.12.1

Relevant code or config:

Just branch off the current master branch, let's add few unit tests.

What you did:

There are some problems with assert .toHaveStyle() — the existing unit test yields false positives
https://github.com/testing-library/jest-dom/blob/main/src/__tests__/to-have-style.js#L203-L210

if you change it to 13, it would still pass:

test('can assert against non-existing style in object notation', () => {
  const {queryByTestId} = render(`
      <span data-testid="color-example" style="font-size: 12px">Hello World</span>
    `)
  expect(queryByTestId('color-example')).toHaveStyle({
    fontSize: 13,
  })
})

Also, this test would also pass:

test('handles assertions against CSS which would parse as blank values', () => {
  const {queryByTestId} = render(`
    <span data-testid="color-example">Hello World</span>
  `)
  expect(queryByTestId('color-example')).toHaveStyle(`background-image: zzz`)
})

What happened:

It seems the isSubset() function is matching empty strings vs. empty strings — in the first case, because of CSS being in a plain object, in the second case, because parser wipes zzz and turns it into an empty string.

To fix the second test, it's enouch to add a quick check against a falsy value:

function isSubset(styles, computedStyle) {
  return (
    !!Object.keys(styles).length &&
    Object.entries(styles).every(
      ([prop, value]) =>
+        value && (
        computedStyle[prop] === value ||
        computedStyle.getPropertyValue(prop.toLowerCase()) === value
+        ),
    )
  )
}

But this doesn't solve the first unit test, where we're asserting string with px against a plain object. It's not that trivial.

Reproduction:

Copy-paste the unit tests above into src/__tests__/to-have-style.js, on a fork of a current master branch.

Suggested solution:

1. isSubset() function needs hardening, as described above (at least, a check against value as an empty string)
2. plus the whole feature of PR Changed toHaveStyle to use getPropertyValue instead of accessing the property directly #285 is still incomplete, it needs re-working, the part of matching against CSS-value-as-object

We have the failing unit tests, now it's just plain TDD.

[ydaniv]

I'm also seeing backgroundImage always returning positive, even on empty strings.
Using latest with node: 14.15.4