Allow the display of messages without failing

[vernak2539]

This implements functionality where a message can be used to determine if the usage of console[methodName] should cause an error. Fixes #28.

I'm not 100% confident on the naming, happy to take steer on this.

The call stack is a bit weird when echoed out (see below). Is this something we'd want to fix, or something that already occurs?

$ jest tests/index.test.js
  console.log
      console.error
        my error message that I do not control

          126 |       console[methodName] = (...args) => {
          127 |         newMethod(...args)
        > 128 |         originalMethod(...args)
              |         ^
          129 |       }
          130 |       unexpectedConsoleCallStacks.length = 0
          131 |     })

          at console.error (../../../index.js:128:9)
          at Object.<anonymous>.module.exports (index.js:2:11)
          at Object.<anonymous> (index.test.js:5:30)

      at Object.<anonymous> (tests/index.test.js:83:13)

I've not updated the README yet, but will do it once the direction is correct.

[ValentinH]

Regarding the stack trace, this is unfortunate indeed. It would be nice to find a solution because the point of this feature was to keep the original output of console methods 🙈

[renatodeleao]

+1 for this feature. Currently using silenceMessage as workaround to prevent some tests to fail, but would prefer to explicit pass a list of errors that make tests fail, because the list is so much smaller.

[vernak2539]

I'm really sorry about dropping the ball here. I've had some things going on and wasn't able to try to finish this off. I'll update the naming soon 🤞

Regarding the stack trace, this is unfortunate indeed. It would be nice to find a solution because the point of this feature was to keep the original output of console methods 🙈

Yeah, I'm thinking to change it from how it is now, to checking for the "allowMessage" closer to where the method name is overwritten. Hopefully, this will keep the stack trace how we'd want it. This will diverge from the original implementation discussion, but I'm not sure how we could keep the stack trace with that implementation anyhow.

If someone gets to it first, please go ahead!

[vernak2539]

Just an update. I've looked into the call stack issue, and I'm not sure it is an actual issue. I've had a look at the tests before my change to compare. They output something like:

  console.log
     FAIL  tests/fixtures/assert-failure/index.test.js
      console.assert failure
        ✕ does not throw (5 ms)

      ● console.assert failure › does not throw

        Expected test not to call console.assert().

        If the assert is expected, test for it explicitly by mocking it out using jest.spyOn(console, 'assert').mockImplementation() and test that the warning occurs.

        test message
            at captureMessage (/Users/vernacchia/Coding/jest-fail-on-console/index.js:67:25)
            at console.assert (/Users/vernacchia/Coding/jest-fail-on-console/index.js:81:7)
            at Object.<anonymous>.module.exports (/Users/vernacchia/Coding/jest-fail-on-console/tests/fixtures/assert-failure/index.js:2:11)
            at Object.<anonymous> (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/expect/build/toThrowMatchers.js:83:11)
            at Object.toThrow (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/expect/build/index.js:382:21)
            at Object.<anonymous> (/Users/vernacchia/Coding/jest-fail-on-console/tests/fixtures/assert-failure/index.test.js:5:31)
            at Promise.then.completed (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/utils.js:391:28)
            at new Promise (<anonymous>)
            at callAsyncCircusFn (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/utils.js:316:10)
            at _callCircusTest (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/run.js:218:40)

          44 |       const message = errorMessage(methodName, chalk.bold)
          45 |
        > 46 |       throw new Error(`${message}\n\n${messages.join('\n\n')}`)
             |             ^
          47 |     }
          48 |   }
          49 |   const groups = []

          at flushUnexpectedConsoleCalls (../../../index.js:46:13)
          at Object.<anonymous> (../../../index.js:125:7)

    Test Suites: 1 failed, 1 total
    Tests:       1 failed, 1 total
    Snapshots:   0 total
    Time:        0.086 s, estimated 1 s
    Ran all test suites matching /.\/tests\/fixtures\/assert-failure\/index.test.js/i.

For the same test after my changes, the call stack looks the same (minus the change in code)

  console.log
    FAIL tests/fixtures/assert-failure/index.test.js
      console.assert failure
        ✕ does not throw (5 ms)

      ● console.assert failure › does not throw

        Expected test not to call console.assert().

        If the assert is expected, test for it explicitly by mocking it out using jest.spyOn(console, 'assert').mockImplementation() and test that the warning occurs.

        test message
            at captureMessage (/Users/vernacchia/Coding/jest-fail-on-console/index.js:75:25)
            at console.assert (/Users/vernacchia/Coding/jest-fail-on-console/index.js:89:7)
            at Object.<anonymous>.module.exports (/Users/vernacchia/Coding/jest-fail-on-console/tests/fixtures/assert-failure/index.js:2:11)
            at Object.<anonymous> (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/expect/build/toThrowMatchers.js:83:11)
            at Object.toThrow (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/expect/build/index.js:382:21)
            at Object.<anonymous> (/Users/vernacchia/Coding/jest-fail-on-console/tests/fixtures/assert-failure/index.test.js:5:31)
            at Promise.then.completed (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/utils.js:391:28)
            at new Promise (<anonymous>)
            at callAsyncCircusFn (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/utils.js:316:10)
            at _callCircusTest (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/run.js:218:40)

          52 |       }
          53 |
        > 54 |       throw new Error(fullErrorMessage)
             |             ^
          55 |     }
          56 |   }
          57 |   const groups = []

          at flushUnexpectedConsoleCalls (../../../index.js:54:13)
          at Object.<anonymous> (../../../index.js:139:7)

    Test Suites: 1 failed, 1 total
    Tests:       1 failed, 1 total
    Snapshots:   0 total
    Time:        0.096 s, estimated 1 s
    Ran all test suites matching /.\/tests\/fixtures\/assert-failure\/index.test.js/i.

Finally, the call stack for the newly added test is similar as well:

console.log
    STDERR PASS tests/fixtures/allow-message/index.test.js
      console.error display message but not fail (allowMessage)
        ✓ does not throw (16 ms)

    Test Suites: 1 passed, 1 total
    Tests:       1 passed, 1 total
    Snapshots:   0 total
    Time:        0.127 s, estimated 1 s
    Ran all test suites matching /.\/tests\/fixtures\/allow-message\/index.test.js/i.

      at Object.<anonymous> (tests/index.test.js:85:13)

  console.log
    STDOUT   console.error
        Expected test not to call console.error().

        If the error is expected, test for it explicitly by mocking it out using jest.spyOn(console, 'error').mockImplementation() and test that the warning occurs.

        my error message that I do not control
            at console.error (/Users/vernacchia/Coding/jest-fail-on-console/index.js:74:25)
            at Object.<anonymous>.module.exports (/Users/vernacchia/Coding/jest-fail-on-console/tests/fixtures/allow-message/index.js:2:11)
            at Object.<anonymous> (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/expect/build/toThrowMatchers.js:83:11)
            at Object.toThrow (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/expect/build/index.js:382:21)
            at Object.<anonymous> (/Users/vernacchia/Coding/jest-fail-on-console/tests/fixtures/allow-message/index.test.js:5:30)
            at Promise.then.completed (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/utils.js:391:28)
            at new Promise (<anonymous>)
            at callAsyncCircusFn (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/utils.js:316:10)
            at _callCircusTest (/Users/vernacchia/Coding/jest-fail-on-console/node_modules/jest-circus/build/run.js:218:40)
            at processTicksAndRejections (node:internal/process/task_queues:95:5)

          47 |
          48 |       if(typeof allowMessage === 'function' && allowMessage(fullErrorMessage, methodName)) {
        > 49 |         consoleMethodStore.originalMethod(fullErrorMessage)
             |                            ^
          50 |         return;
          51 |       }
          52 |

          at flushUnexpectedConsoleCalls (../../../index.js:49:28)
          at Object.<anonymous> (../../../index.js:134:7)

      at Object.<anonymous> (tests/index.test.js:86:13)

I've updated the code after some of the review. Happy to take more feedback if necessary.

[vernak2539]

@ValentinH ready for a review when you are! Thanks!

[ValentinH]

I just reviewed this PR and I adjusted the code a bit because I don't think it was matching the original intent. Indeed, when using allowConsole, the test was not failing but the logged message wasn't the original one but the one from the lib:

Let me know if this is still matching your needs and then we can merge this 🙂

[vernak2539]

@ValentinH This looks much better than my version! Thanks for taking the time to update it.

I think it will fit the needs still (as the test passes)!

[ValentinH]

Released as https://github.com/ValentinH/jest-fail-on-console/releases/tag/v3.2.0