Add HTTP tests for /source route #2036
Conversation
| afterAll(()=>{ | ||
| return Homebrew.model.deleteMany(); | ||
| }); |
There was a problem hiding this comment.
@AlexeySachkov Um.... this deletes all brews in the database, not just the tested ones. Just ran it on my local install and sure enough, everything was deleted. We definitely don't want this running on the live server.
There was a problem hiding this comment.
Good thing that you have spotted that. Actually, I would expect tests to be connected to a different DB, because they are launched under "test" node environment. Probably I don't fully understand the API to work with Mongo here.
I suggest we block this PR from an accidental merge by explicitly requesting changes
There was a problem hiding this comment.
@calculuschild , sorry that it took so long for me to respond here, but I finally figured it out: what you saw is indeed happening. The reason for that is that even in tests we by default connect to the main database.
In 6a2581e I added config/test.json to override database in test environment.
However, there is still caveat: jest sets NODE_ENV=test only if NODE_ENV is not set yet (link to docs). So, in local environment where you likely have NODE_ENV=local, it won't work.
I wonder what would be the preferred way of handling that? I doubt that ignoring database cleanup in local environment is a good idea, because someone who has local database with their brews won't be happy to see them all removed just by (possibly accidentally) running tests. We could update package.json to include NODE_ENV=test to all test-related commands, but I'm concerned that we might miss some, because there are 4 of them already.
calculuschild
added
the
🔍 R3 - Reviewed - Awaiting Fixes 🔧
|
Do the two new commits fix the requested change? |
|
Still not fixed. |
5e-Cleric
added
Orphan
|
Do we need this route? |
I don't know how often |
|
I'd say we can close this, and maybe start from scratch, what is the issue? we should log it as an issue. |
Makes sense to me. |
This PR introduces a few tests for
/sourceroute, which check that the app is able to correctly retrieve brew sources and send them back to user. It also introduces a very basic mock forGoogleActionsmodule so we can run HTTP tests without actual access to Google Drive servers.