search

duct-framework / duct

Server-side application framework for Clojure
MIT License
1.13k stars 51 forks source link

Missing Ring's defaults middlewares in tests #73

Closed LukasRychtecky closed 5 years ago

LukasRychtecky commented 6 years ago

Hi,

I've found inconsistent behaviour in a test from the template.

When a routes is defined with Compojure (e.g.: https://github.com/LukasRychtecky/peridot-query-params/blob/master/src/peridot_query_params/handler/example.clj) baz and foo query parameters are nil when running test like https://github.com/LukasRychtecky/peridot-query-params/blob/master/test/peridot_query_params/handler/example_test.clj.

When the app is run from Duct (a classic HTTP request) everything works as expected (bar and foo are filled with query parameters). The reason is that Ring's default middlewares https://github.com/duct-framework/module.web/blob/master/src/duct/middleware/web.clj#L119 aren't called in tests (thus params middleware doesn't fill :params key in a request).

What do you think it'd be the best solution? Somehow running tests within :duct.module.web/site component?

Sure routes can be wrapped in wrap-defaults, but I think this woudn't be a good fix.

mariusz-jachimowicz-83 commented 6 years ago

@LukasRychtecky It depends on what granularity do you want. From unit test perspective wrap will be enough. From integration perspective you can launch also server and test behaviour then. You can have both cases in your test. There won't be to much code even with both cases covered. What do you think?

LukasRychtecky commented 6 years ago

@mariusz-jachimowicz-83 I think that current behaviour is unexpected. Because a test example in the template doesn't simulate real behaviour, that's the point. And it could lead a developer into WTF moment (as me).

Manually added middleware in tests is not a good idea, because next time something else could be forgotten (new middleware in future).

weavejester commented 5 years ago

The tests that the template generates are unit tests, rather than integration tests, and therefore lack the full middleware chain. It might be an idea to add in an integration test to the template example, but I don't think it's a bad idea to have unit tests as well.