Body validation is not working

aeremin commented 5 years ago

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

When using https://github.com/nestjs/nest/tree/master/sample/01-cats-app example, doing POST / with empty body doesn't trigger validation/doesn't result in 400 Bad Request error.

Expected behavior

Test fails, as HTTP request returns error 400 (as described in https://docs.nestjs.com/techniques/validation).

Minimal reproduction of the problem with instructions

I have changed create method to

@Post() //@Roles('admin') async create(@Body() createCatDto: CreateCatDto) { //this.catsService.create(createCatDto); }

and added a test case

it(/POST cats, () => { return request(app.getHttpServer()) .post('/cats') .expect(201); });

to https://github.com/nestjs/nest/blob/master/sample/01-cats-app/e2e/cats/cats.e2e-spec.ts

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

Because request validation is importtan and critical feature (and according to documentation it supposed to work).

Environment

Nest version: Cloned repo from https://github.com/nestjs/nest/commit/5e2727bf905cc3a4ca26267c6a385793088ff43e

kamilmysliwiec commented 5 years ago

I just tested this locally and it works fine. Please, provide a minimal repository which reproduces your issue.

aeremin commented 5 years ago

Please see https://github.com/aeremin/nest/commit/a519f02b4482e28f6e93a355593a703a682bc927

Complete list of commands:

git clone https://github.com/aeremin/nest cd nest/sample/01-cats-app npm i npm run e2e

Test passes (but I would expect it to fail)

npm -v 6.9.0

node -v v8.10.0

Please let me know if there is anything else I could provide to help reproduce it.

kamilmysliwiec commented 5 years ago

Tests aren't using ValidationPipe, see https://github.com/aeremin/nest/blob/a519f02b4482e28f6e93a355593a703a682bc927/sample/01-cats-app/e2e/cats/cats.e2e-spec.ts#L12-L21 Add:

app = module.createNestApplication();
app.useGlobalPipes(new ValidationPipe());

aeremin commented 5 years ago

I see, I actually just figured it on my own (https://github.com/aeremin/nest/commit/52844676dc46c1cca5dbe27564b43b9ca680b32a). Is there any recommended way of doing complete e2e testing then (i.e. one which will cover the code in bootstrap())?

Thanks for help!

kamilmysliwiec commented 5 years ago

Create a helper function which takes app as an argument and applies configuration (pipes etc).

lock[bot] commented 5 years ago

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.

nestjs / nest