Allow creating a testing handler for HTTP/RPC/WS controllers

[VinceOPS]

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?
See #3938

[ ] 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

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

[coveralls]

Pull Request Test Coverage Report for Build 4e598554-7b3c-4414-85b4-3e7323b07c5f

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 94.575%

---

Totals
Change from base Build c31de5ab-d6ea-4463-9c86-599e16909615:	0.0%
Covered Lines:	5300
Relevant Lines:	5604

---

💛 - Coveralls

[kamilmysliwiec]

Looks great so far!

[VinceOPS]

Thanks @kamilmysliwiec. I did the same work for RPC and WebSocket. Please let me know what could be improved!

I thought about adding a single public static method to Test, which would accept the same additional parameter as createContext ('http' | 'rpc' | 'ws'), in order to chose the Testing Handler to be created. This would allow doing something like this:

await Test
  .createTestingHandler({ class, methodName, protocol: 'http' })`
  .setRequest(fakedRequest)
  .run();

The Return type of createTestingHandler would obviously depend on protocol (either HttpTestingHandler, RpcTestingHandler, or WsTestingHandler). What do you think?

[kamilmysliwiec]

I'll review as soon as possible. Thanks @VinceOPS!

[VinceOPS]

Hey @kamilmysliwiec, I had a look into ExternalContextCreator and thought about creating the context using the static method fromContainer. However, it looks like it would (in the end) require a Module in order for the resolution of dependencies to work. Shall I change the API so it takes a Module (or Testing module) as a parameter?

Thanks

[Green-Cat]

This looks great, thanks!

[kamilmysliwiec]

I'm sorry to keep you waiting @VinceOPS! The PR looks fantastic 🔥

ExternalContextCreator is an injectable class that can be injected anywhere you want https://github.com/nestjs/graphql/blob/master/lib/services/resolvers-explorer.service.ts#L42 (inside the Nest ctx app though).

thought about creating the context using the static method fromContainer

This is somewhat more complicated though. The fromContainer expects the NestContainer instance which in turn is instantiated by the NestFactory before the initialization process.

Perhaps, we should take this idea even further and consider the following use-case - what if someone wants to test the handler in isolation (without spinning up the HTTP server) BUT wants to have all the enhancers bound with { provide: APP_INTERCEPTOR ...etc.. } available as well?

This wouldn't be possible with the current API, so it doesn't really cover all the use-cases.

What if changed this:

await Test
  .createTestingHandler({ class, methodName, protocol: 'http' })`
  .setRequest(fakedRequest)
  .run();

to this:

const moduleRef = await Test.createTestingModule({
  imports: [....] // whatever
}).compile() // classic approach

await moduleRef
    .createTestingHandler({ class, methodName, protocol: 'http' })`
    .setRequest(fakedRequest)
    .run();

The TestingModule (moduleRef) extends the NestApplicationContext which means that you can easily grab the ExternalContextCreator using the this.get(ExternalContextCreator) within the TestingModule class :)

TL;DR; We could move the createTestingHandler method into the TestingModule where you can grab this ExternalContextCreator from the context instead of manually creating it

[VinceOPS]

Thank you for the explanations @kamilmysliwiec 👍!
I'm gonna spend some time on this ASAP. I like the idea of exposing this API via TestingModule 🔥.

[VinceOPS]

Hi @kamilmysliwiec

I moved the code to TestingModule and did set up the context creation. The basic (expected) behavior is good! You can have a look at the 3 test suites.
This it not complete, yet. I want to:

• Refactor the way the context are created. All this new code is making a lot of "noise" in testing-module.ts (any suggestion most welcome). I'm not done yet thinking about it (Are the HTTP/RPC/WS builders still relevant? Where external contexts should be created? etc).
• As you already know and said, the way I'm passing the instance to ExternalContextCreator is not going to work with request-scoped instances
• I made some nasty type-castings here and there 🙈 #WIP
• I am still not able to use @Payload, @Ctx (RPC) and @MessageBody (WS)... It looks like they are not "injected".

At this point, I feel like your insights would be pretty helpful 😬 ! Thanks 🙏

[kamilmysliwiec]

@VinceOPS

I am still not able to use @payload, @ctx (RPC) and @MessageBody (WS)... It looks like they are not "injected".

Can you create tests for this? I'll fix this issue locally then :)

As you already know and said, the way I'm passing the instance to ExternalContextCreator is not going to work with request-scoped instances

Please, pull the latest version of @nestjs/{...} packages. I've added a few additional features that should simplify this process. See comments (I'll add them in a second)

[kamilmysliwiec]

In the latest version, the ModuleRef exposes the introspect() method to check the class scope. Example:

const { scope } = this.get(ModuleRef).introspect(params.class);
if (scope === Scope.REQUEST) {
 // add logic for request-scoped controllers here (I can help with that)
} else {
 // use what you have now
} 

[VinceOPS]

@kamilmysliwiec Heya Kamil! Guess who's back 👻 😂

I know it's been a long time and you probably forgot everything about this PR 😭 🔫 ... Just in case you would find some time for this possible feature, I pushed the test case I could never pass! allows message body to be injected using decorator @MessageBody in ws-testing-handler.spec.ts. It's too bad, because it looks like everything else is working.

There are refactorings that I'm holding for now (e.g. almost duplicated createContext functions in testing-module.ts), but I think it's good enough for you to have a look at the issue of the request-scoped instance (cf the 3 occurences of // TODO: Kamil's gonna do something here, for Request-scoped instances 😬).

Please let me know if I can help 🙏

[kamilmysliwiec]

It seems that WsParamsFactory retrieves undefined value as input arguments (args === undefined) and so it returns null. I don't have too much time atm to investigate it further though. Will take a look asap

[VinceOPS]

Hi @kamilmysliwiec! I had a look into it tonight and it looks like the callback created by createPipesFn (fnApplyPipes) indeed provides null as data to my websocket gateway method 😕

    @WebSocketGateway()
    class EventsGateway {
      @SubscribeMessage('identity')
      async identity(@MessageBody() data: string): Promise<string> {
        return data;
      }
    }

There's one thing bugging me here... As far as I understood, MessageBody here is supposed to add a WS Param type PAYLOAD (3) to my parameter... but somewhat, in the process, this is lost OR something does not care? 😂

I don't know the "context creator" enough to understand.

---

Hitting a breakpoint on this line (ExternalContextCreator.createPipesFn):

        args[index] = await this.getParamValue(
          value,
          { metatype, type, data },
          pipes.concat(paramPipes),
        );

I can see that paramsOptions is an Array(1) with the item inside being:

{
  index: 0,
  extractValue: (...args) => paramsFactory.exchangeKeyForValue(numericType, data, args),
  type: 3,
  data: undefined,
  pipes: [],
  metatype: function String() { [native code] },
}

paramsOptions[0].metatype is '', and paramsOptions[0].extractValue() is null.

---

Regarding this line (just before):

const value = extractValue(...params);

value is obviously null and params is [undefined, "Hello World"].

Does it help? 🤯

Thanks!

[kamilmysliwiec]

I've looked into this issue once again and I can see that WsParamsFactory#exchangeKeyForValue receives undefined as args because ExternalContextCreator passes them as a third argument, while WsParamsFactory only takes 2:

So in fact, data (which is undefined) = our args array.

Funny enough, this has changed in 8.0.0 so once you merge 8.0.0 branch in, the issue will be fixed (just tested) :)

One thing to keep in mind - remember to import from @nestjs/{..} while referencing other packages:

(testing-module.ts above^)

[VinceOPS]

Sounds great 😍

Gonna get back to it ASAP 😄! Thanks! I will keep you posted.

[VinceOPS]

Thank you @kamilmysliwiec 🙏🏼 ! It's indeed working when rebased on branch 8.0.0 🎉

I have troubles using Pipes (test executes Pipes passed as classes in @Query) when passing a class and not an instance (it's working with an instance 👍🏼). I assume this is just me missing something. I tried to add the pipe class to my providers but it didn't change anything. Will have a second look at it ASAP.

[kamilmysliwiec]

@VinceOPS There might be several different reasons why this is happening. Maybe check if this line:

const module = this.getContextModuleName(instance.constructor);

in ExternalContextCreator#create gives you any reasonable output (module is required to retrieve pipe instances later on in the PipesContextCreator).

[yossarin]

Any news on this ?

[Nisitay]

@VinceOPS What's the status of this?

[elton-okawa]

Hi @kamilmysliwiec, I'd like to continue developing this feature, but I'd need some help and clarification to get started

Are those changes still relevant for nestjs 10? What are the next steps?