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

OverrideMiddleware is missing #4073

Closed
vanhumbeecka opened this issue Feb 17, 2020 · 9 comments · May be fixed by #12541
Closed

OverrideMiddleware is missing #4073

vanhumbeecka opened this issue Feb 17, 2020 · 9 comments · May be fixed by #12541

Comments

@vanhumbeecka
Copy link

Feature Request

Is your feature request related to a problem? Please describe.

While trying to write tests for NestJS, I'm trying to override nestjs-middleware, by defining it with overrideProvider(MyMiddleware).useValue({ use(req, res, next) { next() }).
This doesn't work however, since he just keeps dropping in the 'real' middleware (MyMiddleware) instead of the overridden one.

Describe the solution you'd like

Just like overrideProvider or overrideInterceptor, it would be useful to have a similar overrideMiddleware function for testing purposes.

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

Better/easier testability of NestJs applications!

@vanhumbeecka vanhumbeecka added needs triage This issue has not been looked into type: enhancement 🐺 labels Feb 17, 2020
@kamilmysliwiec kamilmysliwiec added scope: testing and removed needs triage This issue has not been looked into labels Feb 17, 2020
@kamilmysliwiec
Copy link
Member

For anyone looking for a workaround - you can simply override the module class and provide a different configure() method

@tkskumar
Copy link

tkskumar commented Feb 1, 2021

I didn't get your comment @kamilmysliwiec , like for below example how can i do ?

  beforeEach(async () => {
    const moduleRef = await Test.createTestingModule({
      imports: [AppModule],
    })
      .overrideInterceptor(AuthMiddleware)
      .useValue({ use: (req: Request, _: Response, next: NextFunction) => next() })
      .compile();

    app = moduleRef.createNestApplication();
    await app.init();
  });

@micalevisk micalevisk moved this to 🤔 I'll investigate later in My contributions to NestJS framework 😻 Dec 17, 2021
@micalevisk micalevisk moved this from 🤔 I'll investigate later to 💭 Thinking on take this in My contributions to NestJS framework 😻 Dec 17, 2021
@micalevisk
Copy link
Member

@tkskumar would be something like this:

app.e2e-spec.ts
describe('AppController (e2e)', () => {
  let app: INestApplication

  beforeAll(async () => {
    const moduleRef = await Test.createTestingModule({
      imports: [AppModule],
    }).compile()

    app = moduleRef.createNestApplication()

    const appModule = app.get(AppModule)
    appModule.configure = function(consumer: MiddlewareConsumer) {
      consumer
        .apply((req, res, next) => { next(); }) // <<<<<<<
        .forRoutes('bar')
    }

    await app.init()
  })
})

Or you could use manual mocks from Jest to override the middleware class, like:

Full working example

Given the following project

.
├── nest-cli.json
├── package.json
├── package-lock.json
├── README.md
├── src
│   ├── app.controller.ts
│   ├── app.module.ts
│   ├── main.ts
│   ├── __mocks__
│   │   └── my.middleware.ts
│   └── my.middleware.ts
├── test
│   ├── app.e2e-spec.ts
│   └── jest-e2e.json
├── tsconfig.build.json
└── tsconfig.json
src/app.module.ts
import { Module, NestModule, MiddlewareConsumer } from '@nestjs/common'
import { AppController } from './app.controller'
import { MyMiddleware } from './my.middleware'

@Module({
  controllers: [AppController],
})
export class AppModule implements NestModule {
  configure(consumer: MiddlewareConsumer) {
    consumer
      .apply(MyMiddleware)
      .forRoutes('bar')
  }
}
src/my.middleware.ts
import { NestMiddleware } from '@nestjs/common'
export class MyMiddleware implements NestMiddleware {
  use(_, __, next) {
    console.time('request took')
    next();
    console.timeEnd('request took')
  }
}
src/__mocks__/my.middleware.ts
import { NestMiddleware } from '@nestjs/common'
export class MyMiddleware implements NestMiddleware {
  use(_, __, next) {
    console.time('Looks like the request took')
    next();
    console.timeEnd('Looks like the request took')
  }
}
src/app.controller.ts
import { Controller, Get } from '@nestjs/common'

@Controller()
export class AppController {
  @Get('foo')
  getFoo() {
    return 'foo'
  }
  @Get('bar')
  getBar() {
    return 'bar'
  }
}
test/app.e2e-spec.ts
import * as request from 'supertest'
import { Test } from '@nestjs/testing'
import { AppModule } from './../src/app.module'
import { INestApplication } from '@nestjs/common'

jest.mock('../src/my.middleware.ts')

describe('AppController (e2e)', () => {
  let app: INestApplication

  beforeAll(async () => {
    const moduleRef = await Test.createTestingModule({
      imports: [AppModule],
    }).compile()

    app = moduleRef.createNestApplication()

    await app.init()
  });

  it('GET /foo', () => {
    return request(app.getHttpServer())
      .get('/foo')
      .expect(200)
      .expect('foo')
  })

  it('GET /bar', () => {
    return request(app.getHttpServer())
      .get('/bar')
      .expect(200)
      .expect('bar')
  })

  afterAll(async () => {
    await app.close()
  })
})

@leonardovillela
Copy link
Contributor

Hi everyone, good afternoon.

I'm working on that to add this functionality to the current testing module capabilities.

@fauxbytes
Copy link

@micalevisk is all that boilerplate actually required? Couldn't one get away with:
app.get(AppModule).configure = () => {} ?

@micalevisk
Copy link
Member

is all that boilerplate actually required?

There is no difference between yours and mine, you've just removed that temporary variable.

@schiemon
Copy link

schiemon commented Oct 9, 2023

Hi everyone,

I implemented the desired overrideMiddleware method in #12541! Maybe some maintainer (cc @kamilmysliwiec) can take a look at it.

@kamilmysliwiec
Copy link
Member

Let's track this here #12541

@github-project-automation github-project-automation bot moved this from 💭 Maybe I'll take this to 😔 abandoned in My contributions to NestJS framework 😻 Oct 23, 2023
schiemon added a commit to schiemon/nest that referenced this issue Oct 23, 2023
@nievelstone
Copy link

What is the status of this issue? @schiemon @kamilmysliwiec

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.

8 participants