Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem installing spec reporter with Angular 12 and mocha #838

Closed
dpraul opened this issue Jul 15, 2021 · 11 comments · Fixed by #843
Closed

Problem installing spec reporter with Angular 12 and mocha #838

dpraul opened this issue Jul 15, 2021 · 11 comments · Fixed by #843

Comments

@dpraul
Copy link
Contributor

dpraul commented Jul 15, 2021

Hi there! I am in the process of moving our application from Angular 11 to Angular 12. Since updating, we now get the following error from ng-mocks when I try to run our unit-testing suite:

Uncaught Error: ng-mocks cannot install own spec reporter. This affects its core features for MockInstance and MockRender. Please report an issue on github. If you use jest v27, please add to its config testRunner=jest-jasmine2 for now and upvote the issue on github: https://github.com/facebook/jest/issues/11483.
at http://localhost:9876/_karma_webpack_/vendor.js:196649:9

Error: ng-mocks cannot install own spec reporter. This affects its core features for MockInstance and MockRender. Please report an issue on github. If you use jest v27, please add to its config testRunner=jest-jasmine2 for now and upvote the issue on github: https://github.com/facebook/jest/issues/11483.
  at messageCoreChecker (http://localhost:9876/_karma_webpack_/webpack:/node_modules/ng-mocks/cjs/lib/common/ng-mocks-stack.js:119:1)
  at install (http://localhost:9876/_karma_webpack_/webpack:/node_modules/ng-mocks/cjs/lib/common/ng-mocks-stack.js:172:1)
  at Object.23851 (http://localhost:9876/_karma_webpack_/webpack:/node_modules/ng-mocks/cjs/lib/common/ng-mocks-stack.js:178:1)
  at __webpack_require__ (http://localhost:9876/_karma_webpack_/webpack:/webpack/bootstrap:19:1)
  at Object.71794 (http://localhost:9876/_karma_webpack_/webpack:/node_modules/ng-mocks/cjs/index.js:15:1)
  at __webpack_require__ (http://localhost:9876/_karma_webpack_/webpack:/webpack/bootstrap:19:1)
  at Module.4825 (http://localhost:9876/_karma_webpack_/main.js:30055:66)
  at __webpack_require__ (http://localhost:9876/_karma_webpack_/webpack:/webpack/bootstrap:19:1)
  at http://localhost:9876/_karma_webpack_/webpack:/webpack/startup:4:1
  at Function.__webpack_require__.O (http://localhost:9876/_karma_webpack_/webpack:/webpack/runtime/chunk loaded:23:1)

Looking through ng-mocks-stack, I suspect this is because we use mocha as our test reporter, whereas ng-mocks is trying to install reporters for either Jasmine or Jest.

We are running ng-mocks 12.3.1. Here's the output from ng version:

Angular CLI: 12.1.2
Node: 14.15.1
Package Manager: yarn 1.22.5
OS: linux x64

Angular: 12.1.2
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, language-service, material, platform-browser
... platform-browser-dynamic, upgrade

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1201.2
@angular-devkit/build-angular   12.1.2
@angular-devkit/core            12.1.2
@angular-devkit/schematics      12.1.2
@ngtools/webpack                12.1.2
@schematics/angular             12.1.2
rxjs                            6.6.7
typescript                      4.3.5
webpack                         5.42.0
@satanTime
Copy link
Member

Cool. Thanks for posting the issue!

I'll try to fix it ASAP next days. In the worst case the next weekend.

@satanTime
Copy link
Member

Btw, do you have an example/ article how to configure mocha for angular?

@dpraul
Copy link
Contributor Author

dpraul commented Jul 15, 2021

Not sure about any articles, but I can give you what we have! We actually use @angular-builders/custom-webpack to provide a custom karma config to ng test.

Here is, I believe, the bare essentials needed in the karma.conf.js:

const karmaMochaPlugin = require('karma-mocha');
const karmaMochaReporterPlugin = require('karma-mocha-reporter');
const karmaChromeLauncherPlugin = require('karma-chrome-launcher');
const karmaAngularDevkitPlugin = require('@angular-devkit/build-angular/plugins/karma');

module.exports = (config) => {
  config.set({
    frameworks: ['mocha', '@angular-devkit/build-angular'],
    browsers: ['Chrome'],
    plugins: [
      karmaMochaPlugin,
      karmaMochaReporterPlugin, // I'm not sure if the reporter plugin is actually necessary
      karmaChromeLauncherPlugin,
      karmaAngularDevkitPlugin,
    ],
    reporters: ['mocha'],
  });
};

And then we provide it to ng test in angular.json like so:

{
  "projects": {
    "project": {
      "architect": {
        "test": {
          "builder": "@angular-builders/custom-webpack:karma",
            "karmaConfig": "./karma.conf.js"
          }
        }
      }
    }
  }
}

@dpraul
Copy link
Contributor Author

dpraul commented Jul 20, 2021

Took a crack at this. Made the following changes to ng-mocks-stack:

const installMochaReporter = () => {
  try {
    mocha.setup({
      rootHooks: {
        afterAll: stackPop,
        beforeAll: () => stackPush('root'),
        afterEach: stackPop,
        beforeEach: () => stackPush('test'),
      },
    });

    return true;
  } catch (err) {
    return false;
  }
};

const install = () => {
  if (!ngMocksUniverse.global.has('reporter-stack-install')) {
    let installed = false;
    installed = installJasmineReporter() || /* istanbul ignore next */ installed;
    installed = installJestCircus() || /* istanbul ignore next */ installed;
    installed = installMochaReporter() || /* istanbul ignore next */ installed;
    // istanbul ignore if
    if (!installed) {
      messageCoreChecker();
    }

    ngMocksUniverse.global.set('reporter-stack-install', true);
  }

  return ngMocksUniverse.global.has('reporter-stack-install');
};

All of our tests run, and pass! It doesn't look like mocha exposes suite-level reporting, so not sure if it'll be a deal-breaker to not have that granulatiy.

I'm happy to make a PR with those changes - that said, I'm not super confident it's correct. I tried our test suite without adding any "reporter stack" at all... I created a file with the following and imported it at the beginning of the test bundle file (before any other imports of ng-mocks):

import ngMocksUniverse from 'ng-mocks/cjs/lib/common/ng-mocks-universe';
ngMocksUniverse.global.set('reporter-stack-install', true);

All of our tests also run and pass, despite many calls to MockBuilder and MockRender. Maybe we never hit any conditions where the reporter stack is necessary?

@satanTime
Copy link
Member

Hi,

thank you for the update. The changes look good to me, feel free to create a PR, a contribution is very welcome.
I guess, It will affect coverage due to missing mocha runner. To fix it you need to add // istanbul ignore next before the line with const installMochaReporter.


Regarding the report stack and its usage, I also agree with you. I was thinking about the issue last days, and found out that it is a stupid limitation for people who don't use it, or reset everything properly in afterEach and afterAll.

Because of that, I have an idea to add a way to shift the stack manually. So people, who use unsupported test runners could still rely on this behavior. And the error would be throws only if they forgot to reset the things, so it wouldn't be a blocker anymore.

// adds a stack level
beforeAll(() => ngMocks.rememberStack());
// adds a stack level
beforeEach(() => ngMocks.rememberStack());
// ...
// resets beforeEach level
afterEach(() => ngMocks.clearStack());
// resets beforeAll level
afterAll(() => ngMocks.clearStack());

What do you think, would it work for you?
I just need solve the second problem of computer science regarding proper names for the methods :)

@satanTime
Copy link
Member

satanTime commented Jul 20, 2021

The things I would like to ask upfront:

  • commit message: feat(core): internal stack integration with mocha runner #838
  • the part with catch can be without the error: } catch {

@dpraul
Copy link
Contributor Author

dpraul commented Jul 20, 2021

What do you think, would it work for you?

As I was poking around, I had the same thought - it seemed like this would've been fairly easy for me to have integrated myself if the methods in ng-mocks-stack were exported. I think it's also better to be able to reset things yourself - sure, it's easier to forget to do so, but it also makes it easier to reset only when you need to.

Perhaps you could take the approach that sinon does, where you simply have to call .restore() in some after function to restore any fakes/stubs/spies created. We've been using sinon for a while and find this to be a very elegant approach.

dpraul added a commit to dpraul/ng-mocks that referenced this issue Jul 20, 2021
Add mocha runner support to the internal stack.

Closes help-me-mom#838
dpraul added a commit to dpraul/ng-mocks that referenced this issue Jul 21, 2021
@satanTime
Copy link
Member

Hi @dpraul,

might you provide an example of unit test you have?
I'm trying to configure e2e on CI to cover mocha and a bit stuck how to write a test for mocha without jasmine.

@satanTime
Copy link
Member

v12.4.0 has been released and contains a fix for the issue. Feel free to reopen the issue or to submit a new one if you meet any problems.

@dpraul
Copy link
Contributor Author

dpraul commented Jul 26, 2021

Sure thing! It's fairly simple after the karma configuration above, but the biggest gotcha is that mocha doesn't provide any assertion library, so that needs to be added separately. We use chai. Here's an example with chai:

import { expect } from 'chai';

describe('suite', () => {
  it('should be true', () => {
    expect(true).to.be.true;
  });
});

That said, if you don't want to add an assertion library just for CI you can also write tests using done, like so:

describe('suite', () => {
  it('should be true', (done) => {
    if (true) {
      done();
    } else {
      done('test failed!');
    }
  });
});

Hope that helps!

@satanTime
Copy link
Member

Aha, thanks. I'll give it a try. I think in my case it was failing with an error about missing webpack...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants