search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

jest-console error create friction for plugin authors #17072

Open iandunn opened 5 years ago

iandunn commented 5 years ago

Problem

Even with wp-scripts, the process of setting up testing tools is error prone, and there's a lot of trial-and-error involved. That process is made harder than it needs to be because jest-console throws cryptic errors when devs do normal things, like using console.log().

Because the error messages are cryptic and unexpected, it can easily confuse a developer into thinking that they haven't configured the testing tools correctly, or that there's something wrong with their tests.

In reality, the tools and test are fine, but jest-console is just being very opinionated. In order to understand that, though, you have to dig through Gutenberg's source code.

Proposed Solution

  1. The error messages should be much more clear. e.g., Expected mock function not to be called but it was called with: -> console.log() should not be used unless explicitly expected, see gutenberg/packages/jest-console/README.md for details.
  2. jest-console should be disabled in the default wp-scripts config. It's fine for Gutenberg core, but I think it's too opinionated for plugins.
gziolo commented 5 years ago

Yes, I agree that the messages printed to the console should get improved, e.g. https://github.com/WordPress/gutenberg/blob/1a6d1b7de833b236bef67eedf8164a14364920f0/packages/jest-console/src/matchers.js#L21

This is the easiest part 😃

Is “console.log” an issue here because it’s very likely that folks would use it for debugging purposes? If that’s the case we could make this test failures optional and enable them only in Gutenberg. However, we might also add a hint that if you want to use it in you test you can swallow test failure by calling a corresponding “expect( console ).toHaveLogged()” statement.

iandunn commented 5 years ago

Yeah, that was the reason, and those sound like good solutions to me.

Personally prefer having it off-by-default for plugins, because adding an extra expect() -- and having to look up the exact naming, etc -- every time I use a quick console would slow me down a lot, and feel frustrating and unnecessary.

Having said that, improving the error messages and suggesting the expect would still be a big leap forward.

dyst5422 commented 4 years ago

As someone using this completely unrelated to Gutenberg, having a configuration option that could be changed on a per file or per test basis would be really useful. There are cases when it may log when a test is run in isolation but NOT LOG when running a bunch of tests in a row. (I have a caching function with warning logging in it that I reuse between tests to avoid the overhead)