feat(core): support route injection

[MyAeroCode]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

summary :

Currently, even if the router implementation changed, the new impl is ignored.

Due to the above reasons, the following e2e test fail :

import { Controller, Get, INestApplication, Query } from "@nestjs/common";
import { Test, TestingModule } from "@nestjs/testing";
import supertest from "supertest";

@Controller("api")
class TestController {
    @Get("hello")
    getHello(@Query("userName") userName: string) {
        return `Hello, ${userName}!`;
    }
}

describe("mocking route", () => {
    const userName = "Lutz";

    let moduleRef: TestingModule;
    let controller: TestController;
    let app: INestApplication;
    let agent: supertest.SuperTest<supertest.Test>;

    beforeEach(async () => {
        const testingModuleBuilder = Test.createTestingModule({
            controllers: [TestController],
        });
        moduleRef = await testingModuleBuilder.compile();
        controller = moduleRef.get(TestController);
        app = moduleRef.createNestApplication();
        agent = supertest(app.getHttpServer());

        jest.clearAllMocks();
    });

    test("spyOn", async () => {
        jest.spyOn(controller, "getHello");
        await app.init();

        const res = await agent.get("/api/hello").query({ userName });
        expect(res.status).toBe(200);
        expect(res.text).toBe(`Hello, ${userName}!`);

        //
        // fail from here.
        // reason: new impl (spy function) not called.
        expect(controller.getHello).toBeCalledTimes(1);
        expect(controller.getHello).toHaveBeenNthCalledWith(1, userName);
    });
});

What is the new behavior?

I suggest that if the routing function is overwritten, use the injected one rather than the original one.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

[coveralls]

Pull Request Test Coverage Report for Build d3899006-e65d-4975-bdbf-3fc8f301473a

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 94.61%

---

Totals
Change from base Build acf4277a-2fec-411b-825e-4f7bfdb58a26:	0.001%
Covered Lines:	5108
Relevant Lines:	5399

---

💛 - Coveralls

[jmcdo29]

jest.spyOn doesn't mention changing the original method so that you can do originalObject.originalInstance. If this does work, like you say it does, can you add a test to show it?

Otherwise, can you check this test instead in your repo:

import { Controller, Get, INestApplication, Query } from "@nestjs/common";
import { Test, TestingModule } from "@nestjs/testing";
import supertest from "supertest";

@Controller("api")
class TestController {
    @Get("hello")
    getHello(@Query("userName") userName: string) {
        return `Hello, ${userName}!`;
    }
}

describe("mocking route", () => {
    const userName = "Lutz";

    let moduleRef: TestingModule;
    let controller: TestController;
    let app: INestApplication;
    let agent: supertest.SuperTest<supertest.Test>;

    beforeEach(async () => {
        const testingModuleBuilder = Test.createTestingModule({
            controllers: [TestController],
        });
        moduleRef = await testingModuleBuilder.compile();
        controller = moduleRef.get(TestController);
        app = moduleRef.createNestApplication();
        agent = supertest(app.getHttpServer());

        jest.clearAllMocks();
    });

    test("spyOn", async () => {
        const controllerSpy = jest.spyOn(controller, "getHello");
        await app.init();

        const res = await agent.get("/api/hello").query({ userName });
        expect(res.status).toBe(200);
        expect(res.text).toBe(`Hello, ${userName}!`);

        //
        // fail from here.
        // reason: new impl (spy function) not called.
        expect(controllerSpy).toBeCalledTimes(1);
        expect(controllerSpy).toHaveBeenNthCalledWith(1, userName);
    });
});

[MyAeroCode]

It is not written in the official API document, but it seems to work in that way.
And I think it makes more sense for the changed function to be executed if the routing function is changed, not just in the case of mock.

I did the test in my repo.
The testing code can be found here.
Also you can check the test results on GitHub Action.

If there is another problem, please let me know.
Thank you.

[jmcdo29]

Sure enough it does seem an undocumented functionality. Seems a bit weird, to not use the returned spy object.

Does this change affect other testing libraries, like sinon, and tap? It'd be better to make a change that doesn't enforce people to use one testing library.

[MyAeroCode]

I agree with what you are worried about. But, There will be little impact on other testing libraries.
There is a possibility that the previously successful router mocking test will fail, but there is no force to use only one testing library.

To show this, I did same tests with a various combinations of mocking libaray and test runner.

Mocking library :

• jest built-in mock module
• jasmine built-in mock module
• sinon
• mockito
• testdouble

Test runner :

• jest
• jasmine
• mocha
• ava
• tape
• tap
• qunit

At least for the famous testing library written above, it ensures to work.
The test result results can be found Github Action.

[kamilmysliwiec]

LGTM