search

mikro-orm / nestjs-realworld-example-app

Example real world backend API built with NestJS + MikroORM
https://realworld.io/
MIT License
270 stars 70 forks source link

Fixed Swagger GUI page #102

Open popov654 opened 5 months ago

popov654 commented 5 months ago

Made POST body schemas actually work

B4nan commented 5 months ago

this apparently breaks the tests pretty badly

B4nan commented 5 months ago

i'd guess its gonna be this change

https://github.com/mikro-orm/nestjs-realworld-example-app/pull/102/files?w=1#diff-8d9225643f0fb2cba6a02345b85424c6ce325e67c6512326c659b83e85ff57e5R46

popov654 commented 5 months ago

this apparently breaks the tests pretty badly

I guess something else has gone wrong here, because I ran the tests locally on this branch, and it is just fine.

B4nan commented 5 months ago

I ran the tests locally on this branch

What tests? The failure is from e2e test suite, not from the unit tests.

popov654 commented 5 months ago

I ran the tests locally on this branch

What tests? The failure is from e2e test suite, not from the unit tests.

Ah, got it. OK, I'll try to figure out how to fix this. Unfortunately, for now I have no idea why does not my current code work. The swagger.json schema looks right

popov654 commented 5 months ago

Can you please help me with this thing? I'm quite new to Swagger and testing in Node.

B4nan commented 5 months ago

Did you try reverting that change I noted? It feels like the only thing you changed that can have runtime effect.

B4nan commented 5 months ago

there are actually more changes like that, basically all the removed parameters from @Body

B4nan commented 5 months ago

if that wont help, i cant really help. i dont do webapps anymore, i don't use nest, and i never used swagger either.

also i don't consider this as something very important, this is an example app, its purpose is to show how to work with MikroORM and nest, not how to setup nest with swagger - that part is not that important. i would love to have it there, but its not a requirement really.

popov654 commented 5 months ago

I'm just trying to learn using this app, since it's really short and simple. I found one more place to fix in e2e/Conduit.postman_collection.json file, but even after that it keeps failing. What is the most strange is that I have the following error:

Register
POST https://conduit.productionready.io/api/users [422 Unprocessable Entity, 932B, 647ms]

But when I use Swagger UI I can't get such response at all, no matter what the input is: I get the HTTP 400 Bad Request instead with informative description.

I think what is happening here is really that newman is sending requests NOT to the localhost (the fact that I cannot hit any debug breakpoint kind of confirms this version). I guess this script is not testing the local project, it is testing something else instead.

popov654 commented 5 months ago

I see your point of "just revert it back if it works", but I really don't like the nested approach of { "user": { "username": ..., "email": ..., "password": ... } } because that makes writing @ApiBody() annotation difficult and the annotation itself ugly.

As for "why use the @ApiBody() annotation if it should work without it, when we use classes and not interfaces" - for some reason that just doesn't work for me.

B4nan commented 5 months ago

I think what is happening here is really that newman is sending requests NOT to the localhost (the fact that I cannot hit any debug breakpoint kind of confirms this version). I guess this script is not testing the local project, it is testing something else instead.

Sounds like you are doing something wrong, the e2e test suite fires queries to the local server. I can literally see the query logs on the app instance when I run the e2e test suite.

Should be just about yarn start in one terminal and yarn test:e2e in another.

popov654 commented 5 months ago

Should be just about yarn start in one terminal and yarn test:e2e in another.

Yes, that's exactly what I'm doing. Still no breakpoints are triggered when I run npm run start:debug in the first terminal...

popov654 commented 5 months ago

Then what is the purpose of https://conduit.productionready.io/api URL?

B4nan commented 5 months ago

Then what is the purpose of conduit.productionready.io/api URL?

Where did you find it? I'd guess it's a default, and the yarn test:e2e call overrides it via the APIURL env var:

https://github.com/mikro-orm/nestjs-realworld-example-app/blob/master/package.json#L16

popov654 commented 5 months ago

and the yarn test:e2e call overrides it via the APIURL env var:

No, it does not work that way. Direct command line argument has higher priority than the env var with the same name.

popov654 commented 5 months ago

Again, the readme file in e2e folder says

To locally run the provided Postman collection against your backend, execute:APIURL=http://localhost:3000/api ./run-api-tests.sh

So in fact I was passing http://localhost:3000 or http://localhost:3000/api directly to my PowerShell call, and it turns out that was correct. If I did not do it I had an error like "Connect error" or something link that.

popov654 commented 5 months ago

It seems it's finally working now. The problem yesterday was I that forgot to add /api to my API_URL, I guess. Also there is a 100% chance to get HTTP 400 Bad Request error on user register endpoint when the user from the previous test run in not deleted from the database, and the username for the test run is not random (I used a static one).

B4nan commented 5 months ago

No, it does not work that way. Direct command line argument has higher priority than the env var with the same name.

Well, it does work that way on my end, as well as in the CI 🤷

The problem yesterday was I that forgot to add /api to my API_URL,

The env var is called APIURL, not API_URL. You need to pay more attention to the detail :]