Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server.js requires refactoring to be testable #1934

Closed
AlexeySachkov opened this issue Dec 30, 2021 · 2 comments · Fixed by #1959
Closed

server.js requires refactoring to be testable #1934

AlexeySachkov opened this issue Dec 30, 2021 · 2 comments · Fixed by #1959

Comments

@AlexeySachkov
Copy link
Member

This issue is to discuss commit a293f2e, which is a part of my testing initiative #1183.

The thing I would like to achieve is to be able programmatically instance an application without it consuming a port, to test it. For example, see documentation for supertest.

Note: the commit also does something with app-module-path and I suggest to leave that discussion for another issue/PR, because that change is not directly relevant to restructuring code to make it testable.

@calculuschild
Copy link
Member

calculuschild commented Jan 9, 2022

I haven't done much Node testing. Is there a reason we don't want to consume the port? Is there a case where temporarily using a port for testing conflicts with anything else?

Perhaps this is also a good place to outline what kinds of tests we want to cover.

@AlexeySachkov
Copy link
Member Author

I haven't done much Node testing. Is there a reason we don't want to consume the port? Is there a case where temporarily using a port for testing conflicts with anything else?

I can think of a several reasons: you already have a local instance of Homebrewery up and running and at the same time, after implementing some changes you trying to launch API tests.

Personally, I see API testing as some kind of unit-testing which can be done in relative isolation, i.e. without setting up the whole server on a particular port. Therefore, the fact that unit-test consumes a port looks like some kind of an unwanted side effect to me.

I think that outlining the app itself into a separate file looks better from readability/modularity point of view: if you look at supertest example, it requires an app object. If such object is simply exported from a dedicated module - it would look quite clean, IMHO.

Perhaps this is also a good place to outline what kinds of tests we want to cover.

In scope of this particular issue I'm only interested in API testing. By that I mean some automated way of sending predefined requests to the app to check that response is correct. Examples would be: create a new brew, update it, get its source, etc. There could be more complicated use cases like testing a chain of requests.

From my previous experiments I discovered the following issues: missing input validation #1204, unnecessary huge responses #1203. Additionally, such API tests could work as some kind of API documentation (#809) or at least as example of using the API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants