describeWithDOM enhancement

[nfpiche]

Let me know how this looks, wasn't 100% sure on taking describeWithDOM out of index.js

[ljharb]

s/\t/ /g please

[nfpiche]

woops, thought I caught those, will update

[lelandrichardson]

@ljharb eslint should fail on these, right?

[ljharb]

theoretically, but i haven't gotten around to these rules yet, so i don't know how they're configured :-)

[nfpiche]

This is actually why I thought I got them all, the linter wasn't yelling at me anymore, so it doesn't appear to be catching them everywhere, because I was getting warnings in other places

[ljharb]

Other than my comments, this looks awesome.

[ljharb]

This LGTM! It'd be a semver-minor.

[vesln]

in order to eliminate duplication, you could do smth like: (pseudo code)

function testSuite(register, title, fn) {
  describe('(uses jsdom)', () => {
    if (typeof jsdom === 'function') {
      jsdom();
      register(a, b);
    } else {
      // if jsdom isn't available, skip every test in this describe context
      describe.skip(a, b);
    }
  });
}

function only(title, fn) {
  testSuite(describe.only, title, fn);
}

function skip(title, fn) {
  testSuite(describe.skip, title, fn);
}

function describeWithDOM(title, fn) {
  testSuite(describe, title, fn);
}

[vesln]

As I stated in different PRs, I think we should take care of other mocha interfaces as well (out of scope for this pr obviously), but @nfpiche might be interested in exploring

[lelandrichardson]

For context, describeWithDOM will be removed from enzyme entirely in the 2.0 release. The idea behind it (refreshing the global document for each test) is inherently incompatible with React and causes lots of issues. We will be recommending a single document be used for your entire test-suite moving forward.

That said, I'm fine with merging this in for 1.x releases

[lelandrichardson]

i'm confused. Why exactly were these tests needed to be added separately?

[nfpiche]

We didn't technically need it for .skip, but I felt like having it be consistent would be the way to go. We do need a separate run for the .only tests though, otherwise it will skip the rest of the test suite. Happy to change things up a bit to be less verbose if you have an idea though.

[lelandrichardson]

I suppose that's fine. I'm not 100% sure we even need to add tests for describeWithDOM, but since they're already added it's better than nothing.