Always get `Error: Mocha was not loaded`

vassudanagunta commented 3 years ago

I tried both forms of mocha-suppress-logs usage as described in the README. No matter what I do, I get Error: Mocha was not loaded.

Here is the stack trace for first form, mocha --file ...:

Error: Mocha was not loaded
    at suppressLogs (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha-suppress-logs/index.js:26:11)
    at file:///Users/vas/Repos/projects/TextPlainly/TextPlain.js/test/unit/mochaSuppressLogs.js:5:1
    at ModuleJob.run (node:internal/modules/esm/module_job:175:25)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:437:15)
    at async formattedImport (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/esm-utils.js:7:14)
    at async Object.exports.loadFilesAsync (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/esm-utils.js:55:20)
    at async singleRun (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/cli/run.js:362:5)

Here is the stack trace for second form, calling suppressLogs() from inside a context block (in my case suite):

Error: Mocha was not loaded
    at suppressLogs (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha-suppress-logs/index.js:26:11)
    at Suite.<anonymous> (file:///Users/vas/Repos/projects/TextPlainly/TextPlain.js/test/unit/line.test.js:23:9)
    at Object.create (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/interfaces/common.js:148:19)
    at context.suite (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/interfaces/tdd.js:49:27)
    at Suite.<anonymous> (file:///Users/vas/Repos/projects/TextPlainly/TextPlain.js/test/unit/line.test.js:22:5)
    at Object.create (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/interfaces/common.js:148:19)
    at context.suite (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/interfaces/tdd.js:49:27)
    at file:///Users/vas/Repos/projects/TextPlainly/TextPlain.js/test/unit/line.test.js:11:1
    at ModuleJob.run (node:internal/modules/esm/module_job:175:25)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:437:15)
    at async formattedImport (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/esm-utils.js:7:14)
    at async Object.exports.loadFilesAsync (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/esm-utils.js:55:20)
    at async singleRun (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/cli/run-helpers.js:125:3)
    at async Object.exports.handler (/Users/vas/Repos/projects/TextPlainly/TextPlain.js/node_modules/mocha/lib/cli/run.js:362:5)

I am using ES Modules, in case that makes a difference.

AleG94 commented 3 years ago

The module relies on beforeEach and afterEach being globally available in order to suppress the logs.

This doesn't seem to be the case when using suite() and the TDD interface, that's why you're getting the error.

I'm labelling the issue as enhancement to check how compatibility can be extended.

vassudanagunta commented 3 years ago

Thank you.

Quick question, since I wasn't able to find out for myself: "Suppress console output of successful mocha tests" means that the console output of the tested code is suppressed, not the output of Mocha itself, e.g. the reporting of success / green checkmarks, correct?

AleG94 commented 3 years ago

That's correct, mocha reports will not be affected. Only the console output generated by the code under testing will be suppressed if the test succeeds.

vassudanagunta commented 3 years ago

perfect! Would you be doing this enhancement anyway? Otherwise I can convert my tests to the other testing interface.

I'd help you do this but I'm working on another open source project, the project using this :)

AleG94 commented 3 years ago

Your best bet in the short term would be to switch your testing interface. I'm afraid I won't be able to work on it anytime soon.

vassudanagunta commented 3 years ago

I found a Mocha UI-independent approach to make this work for suppressing logs for all tests. I'd be happy to submit as a PR.

But first I am unclear about the behavior of this usage (from the README):

If instead you want to suppress console output for a specific describe or context block:
const suppressLogs = require('mocha-suppress-logs');

describe('Something', () => {
  suppressLogs();

  it('should do something', () => {
    // test code
  });
});

If I do this in three tests, don't the three calls to suppressLogs result in beforeEachCb being registered three times? In other words, as each test is executed, each subsequent test gets N+1 beforeEachCb invocations?

RE Mocha UI-independence:

I looked at the Mocha docs and its source code. TDD and BDD interfaces use the same underlying implementation. Their only differences are the method names.

For example from the source of bdd.js:

    context.before = common.before;
    context.after = common.after;
    context.beforeEach = common.beforeEach;
    context.afterEach = common.afterEach;
    context.run = mocha.options.delay && common.runWithSuite(suite);

and from tdd.js:

    context.setup = common.beforeEach;
    context.teardown = common.afterEach;
    context.suiteSetup = common.before;
    context.suiteTeardown = common.after;
    context.run = mocha.options.delay && common.runWithSuite(suite);

You can compare those entire files, and their tests to confirm.

My gut says one ought not hardcode a dependency to a specific Mocha UI dependent global. Clearly Mocha is abstracts this all away and should support writing plugins without if-else statements covering every UI's synonyms for the "common" interface terms. And I suspect that the following are not public Mocha API calls:

  beforeEach(beforeEachCb);
  afterEach(afterEachCb);

???

Anyway, there is one for the global cases as I mentioned at the top.

AleG94 commented 3 years ago

If I do this in three tests, don't the three calls to suppressLogs result in beforeEachCb being registered three times? In other words, as each test is executed, each subsequent test gets N+1 beforeEachCb invocations?

When suppressLogs is called inside a describe block, it only applies to test cases inside that block so only one beforeEachCb will be registered.

Anyway, I rewrote the package to use the new Root Hook Plugins api as you can see in commit 05c77da. We lose the ability to suppress logs on a per-block basis and at least version 8.0.0 of mocha is now required but compatibility is extended to every interface supported by mocha. Usage is also further simplified.

I believe it's a good compromise, let me know what you think and I'll release it in a few days.

vassudanagunta commented 3 years ago

Anyway, I rewrote the package to use the new Root Hook Plugins

That's what i had done and was ready to submit via PR 😂.

We lose the ability to suppress logs on a per-block basis

Yeah, that's what I encountered too (it's why I asked all those questions about the per-block suppression)

Will you release it soon?

I haven't had a chance to review your changes. Would you want feedback from me if our approaches differ significantly?

AleG94 commented 3 years ago

That's what i had done and was ready to submit via PR 😂.

That's great 😂

Will you release it soon?

I just released version 0.3.0 🎉

Closing the issue, let me know for any problem or suggestion you might have.

vassudanagunta commented 3 years ago

It works like a charm! I also like how much cleaner/simpler your code got. Nice job!

vassudanagunta commented 3 years ago

As for suggestions, you ought to release log-capture.js as a standalone module.

I've been using log-interceptor for another part of my project. It's more sophisticated (e.g. supports nested log suppression calls) but I never used any of that. The one difference that is significant is that it captures all stdout.write calls, not just console.* calls. I'm not sure that's better, though it's nice to have as an option.

In any case there are other uses for log capture / deferral / redirection beyond Mocha. log-capture.js is small, but its nice and its tested and I haven't found any great alternatives, besides log-interceptor, which is no longer maintained, and which doesn't capture each different console log level separately as log-capture does.

AleG94 commented 3 years ago

As for suggestions, you ought to release log-capture.js as a standalone module.

I like the idea and will probably do it in the near future, thank you for the suggestion!

AleG94 / mocha-suppress-logs

Always get `Error: Mocha was not loaded` #2