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

[Bug]: OgmaCoreModule cot configured #106

Closed
jmcdo29 opened this issue Jul 23, 2020 · 0 comments · Fixed by #109
Closed

[Bug]: OgmaCoreModule cot configured #106

jmcdo29 opened this issue Jul 23, 2020 · 0 comments · Fixed by #109
Assignees
Labels
bug 😭 A regression broken what was functioning properly core 📖 Functionality related to the service or module classes

Comments

@jmcdo29
Copy link
Owner

jmcdo29 commented Jul 23, 2020

Bug Report

Currently, if @OgmaLogger() is used in any test the error Error: Expected OgmaCoreModule to be configured by at last one Module but it was not configured within 0ms will appear in the console. While this doesn't fail the test, because it is an UnhandledPromiseRejection it may end up failing in the future.

Input Code

@Injectable()
export class AppService {
  constructor(@OgmaLogger(AppService) private readonly logger: OgmaService) {}

  getHello(): Record<string, string> {
    this.logger.log("Say Hello!");
    return { hello: "world" };
  }
}
describe('AppService', () => {
  let service: AppService;

  beforeEach(async () => {
    const modRef = await Test.createTestingModule({
      providers: [
        AppService,
        {
          provide: 'OGMA_SERVICE:AppService',
          useValue: {
            log: jest.fn(),
          }
        }
      ]
    }).compile();
    service = modRef.get(AppService);
  });

  it('should be defined', () => {
    expect(service).toBeTruthy();
  });
  it('should return { hello: world }', () => {
    expect(service.getHello()).toEqual({ hello: 'world' });
  });
});

Expected behavior

No errors to arise when running unit tests.

Possible Solution

In the OgmaModule's @Module() metadata, imports: [OgmaCoreModule.Deferred] is used. Because of this, and because it is in a decorator, it gets ran immediately, which means the observable there will be fired and possibly end up erroring out. It can probably be removed as the forRoot, forRootAsync, and forFeature all use the imports directly

Environment


@ogma/nestjs-module version: 0.2.0

 
For Tooling issues:
- Node version: 14.5.0  
- Platform:  MacOS 

Others:

@jmcdo29 jmcdo29 self-assigned this Jul 23, 2020
@jmcdo29 jmcdo29 added bug 😭 A regression broken what was functioning properly core 📖 Functionality related to the service or module classes labels Jul 23, 2020
jmcdo29 added a commit that referenced this issue Jul 25, 2020
Due to how static class members are created in JavaScript and ran at import time rather than call
time `OgmaCoreModule.Deferred` was immediately calling
`OgmaCoreModule.externallyConfigured(OgmaCoreModule, 0)` which was causing Jest and the Node REPL to
throw an error on import of `@ogma/nestjs-module`. This has been remedied and verified by unit
tests, integration tests, and manual tests with imports from the REPL itself.

fix #106
jmcdo29 added a commit that referenced this issue Jul 25, 2020
Due to how static class members are created in JavaScript and ran at import time rather than call
time `OgmaCoreModule.Deferred` was immediately calling
`OgmaCoreModule.externallyConfigured(OgmaCoreModule, 0)` which was causing Jest and the Node REPL to
throw an error on import of `@ogma/nestjs-module`. This has been remedied and verified by unit
tests, integration tests, and manual tests with imports from the REPL itself.

fix #106
jmcdo29 added a commit that referenced this issue Jun 20, 2021
Due to how static class members are created in JavaScript and ran at import time rather than call
time `OgmaCoreModule.Deferred` was immediately calling
`OgmaCoreModule.externallyConfigured(OgmaCoreModule, 0)` which was causing Jest and the Node REPL to
throw an error on import of `@ogma/nestjs-module`. This has been remedied and verified by unit
tests, integration tests, and manual tests with imports from the REPL itself.

fix #106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 😭 A regression broken what was functioning properly core 📖 Functionality related to the service or module classes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant