feat: Include hidden elements by default

[eps1lon]

What:

• default for hidden is now true meaning inaccessible roles are included by default
• make the default value configurable

Why:

• it was demanded

How:

• change default value (read from config)
• adjust test descriptions

Checklist:

• Documentation added to the
  docs site
• I've prepared a PR for types targeting
  DefinitelyTyped
• Tests
• Ready to be merged
  

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 338e4d2:

Sandbox	Source
immutable-cache-iljjy	Configuration

[kentcdodds]

Thanks for making this. Just a few notes.

[eps1lon]

I think we should leave it at its current default

-- #390 (comment)

I want to make sure we're in sync here: This changes the default value. This basically restores the behavior to where it was before ByRole got introduced.

[kentcdodds]

I'm starting to think that it's ok to leave this enabled by default. My biggest challenge making this decision is that I no longer have a codebase with a bunch of tests to get a feel for what the real-world runtime of tests will be like. All I have to go off of is from folks like @oliviertassinari and others who are running thousands of tests.

I think that running 2000+ tests in less than a minute is pretty good personally, especially considering the improved confidence you're getting. I'd really love to hear from others though.

[oliviertassinari]

@kentcdodds I think that @eps1lon has one of the best positions to provide a great answer this tradeoff. I have personally too limited context. I can only relate what I have experienced. My observations with Material-UI's have been: 1. performance slowness comes from style computation (window.getComputedStyle) with JSDOM. 2. style computation output doesn't impact the results of Material-UI tests (yet).

[kentcdodds]

I think you're right.

@eps1lon, I defer to you. What do you suggest we do?

[eps1lon]

I wasn't a fan of introducing the original change as non-breaking and would therefore not introduce another one (we already got two reports). I'd say we keep the old default and make it configurable so that folks who actually measure a non-tolerable performance hit can deactivate this feature. But I wouldn't advertise it too much. Usually we should stay below 100ms cost which is perfectly fine.

For folks running their whole test suite while working I can only recommend not doing that since it penalizes additional tests which is not a situation you want to be in. jest has an awesome cli to help with these issues (mocha not so much) and both have the option to put (ir)relevant tests on a allow/disallow list.

[kentcdodds]

Sounds great. Let me know when you have something for me to review/merge and we'll make it happen. 👍

[eps1lon]

Sounds great. Let me know when you have something for me to review/merge and we'll make it happen.

#398 extracts the configure feature from this PR.

[kentcdodds]

Thanks @eps1lon 👍