Errors in beforeEach fail silently

[rjcorwin]

I'm submitting a...

[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

If you have an error in your beforeEach code in unit tests, the error will throw silently and your tests will fail because things like controllers have not been instantiated. You kind of "smell" this when your tests are complaining about trying to call methods on properties of undefined.

Expected behavior

We would expect to see an error in our test output.

Minimal reproduction of the problem with instructions

Using the default app template, place a throw in the beforeEach and note how the test will give feedback that the test failed on the assertion for a different reason, not in beforeEach because of a throw.

import { Test, TestingModule } from '@nestjs/testing';
import { AppController } from './app.controller';
import { AppService } from './app.service';
import { SharedModule } from './shared/shared.module';

describe('AppController', () => {
  let appController: AppController;

  beforeEach(async () => {
    const app: TestingModule = await Test.createTestingModule({
      controllers: [AppController],
      providers: [AppService]
    }).compile();

    throw new Error('Yo!')

    appController = app.get<AppController>(AppController);
  });

  describe('root', () => {
    it('should return "Hello World!"', () => {
      expect(appController.getHello()).toBe('Hello world!');
    });
  });
});

What is the motivation / use case for changing the behavior?

It's common for test modules to fail compiling because of DI reasons, thus a lot of beginners who don't learn how to "smell" this issue will be at a loss and potentially give up on testing.

Environment

Nest version: 6.0.0

For Tooling issues:

• Node version: 9.11.1
• Platform: Mac 10.14.3

Potentially related issue

Tests are run even when beforeAll throws error #2713

[kamilmysliwiec]

I don't think that this issue is related to Nest framework, but rather to the testing framework (in this case jest), right?

[rjcorwin]

Hi @kamilmysliwiec - Thanks for the reply. The root of the issue is in the default test runner for Jest known as Jasmine. Jest is phasing out Jasmine in favor of available but not yet default Jest test runner known as Jest Circus. Reading through the Jest issue queue I can see here is the issue were we can follow when Jest Circus will become the default test runner.

While Jest Circus is being figured out, perhaps the developer friendly thing to do would be to modify the default spec file templates to follow the suggestion of encapsulating beforeEach contents with a try that when caught, logs the error message and runs process.exit(1). It's not pretty, but if it's a common gotcha of the beforeEach throwing errors (it is for me because I mess up DI things often), then it would be worth it.

[kamilmysliwiec]

I fully understand your concerns. However, it doesn't make too much sense to modify every existing sample because there is an issue in the test runner. Once they solve this issue, we would have to revert those changes back :(

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.