Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Analysis of weird test failure #2

Open
pedrottimark opened this issue Mar 5, 2019 · 3 comments
Open

Analysis of weird test failure #2

pedrottimark opened this issue Mar 5, 2019 · 3 comments

Comments

@pedrottimark
Copy link

@SimenB /cc @thymikee The regression is caused by 2 bugs in jsdom package:

  1. In Check for existence of Element when comparing DOM-nody-things (#7786) (clean cherry pick version) jestjs/jest#7995 we changed isDomNode function in jasmineUtils.ts file:

    • from duck type test: obj !== null && typeof obj === 'object' && typeof obj.nodeType === 'number' && typeof obj.nodeName === 'string'
    • to instanceof test: typeof Node !== 'undefined' && obj instanceof Node
    const fragCreate = document.createDocumentFragment();
    const fragSimple = JSDOM.fragment();
    
    fragCreate instanceof Node // true
    fragSimple instanceof Node // false
    
    fragCreate instanceof DocumentFragment // true
    fragSimple instanceof DocumentFragment // false
    
    Object.prototype.toString.call(fragCreate) // '[object DocumentFragment]'
    Object.prototype.toString.call(fragSimple) // '[object DocumentFragment]'
    
    typeof fragCreate.isEqualNode // 'function'
    typeof fragSimple.isEqualNode // 'function'
  2. Serializing a fragment causes side effect enumerable symbol Symbol(SameObject caches)

    • When you commented out toMatchDiffSnapshot then both fragment instances have no enumerable properties, so test passes as false negative, even if I comment out span2.innerHTML = 'hello' to compare <span>hello</span> to <span>huh</span>

    • With toMatchDiffSnapshot then first has been serialized, so has bogus implementation symbol key, but because of reassignment to second the new instance has no keys

    • To confirm, if I call pretty-format with DOMElement plugin to serialize the second instance, then toEqual passes as false negative, because both have value of SameObject caches

@pedrottimark
Copy link
Author

Here is a hypothesis for y’all to evaluate:

  • Jest default --env=jsdom injects one set of DOM classes into the test environment
  • const {JSDOM} = require('jsdom'); to access the fragment method creates a second set of DOM classes
const {JSDOM} = require('jsdom');

test('classes', () => {
  const span1 = document.createElement('span');
  span1.innerHTML = 'hello';

  const frag = JSDOM.fragment('<span>hello</span>');
  const span2 = frag.children[0];

  expect(span1.constructor.name).toBe('HTMLSpanElement');
  expect(span1 instanceof Node).toBe(true);

  expect(span2.constructor.name).toBe('HTMLSpanElement');
  expect(span2.constructor === span1.constructor).toBe(false);
  expect(span2 instanceof Node).toBe(false);
});

@pedrottimark
Copy link
Author

What do you think of a solution that we discussed in jestjs/jest#7791 (comment)

function isDomNode(obj) {
  return (
    obj !== null &&
    typeof obj === 'object' &&
    typeof obj.nodeType === 'number' &&
    typeof obj.nodeName === 'string' &&
    typeof obj.isEqualNode === 'function'
  );
}

@pedrottimark
Copy link
Author

Here is matchers-dom-fragment.test.js with 4 regression tests that fail in 24.2.0-alpha.0:

/* @jest-environment jsdom*/
/* eslint-env browser*/
const {JSDOM} = require('jsdom');

describe('JSDOM fragment', () => {
  describe('without side effect', () => { /* pass */ });
  describe('with side effect', () => {
    test('isNot false', () => {
      const markup = '<strong>deep</strong><span>equal</span>';
      const a = JSDOM.fragment(markup);
      const b = JSDOM.fragment(markup);

      expect(a.children.length).toBe(2); // cause side effect

      expect(a).toEqual(b);
      expect(b).toEqual(a);
    });

    test('isNot true', () => {
      const a = JSDOM.fragment('<strong>not</strong><span>deep equal</span>');
      const b = JSDOM.fragment('<span>not</span><span>deep equal</span>');

      expect(a).not.toEqual(b);
      expect(b).not.toEqual(a);
    });
  });
});

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant