✅ Test Onigumo.Downloader.main #139
Conversation
The application - starts the HTTPClient, - downloads the URLs.
test/onigumo_test.exs
Outdated
| @@ -9,6 +9,22 @@ defmodule OnigumoTest do | |||
|
|
|||
| setup(:verify_on_exit!) | |||
|
|
|||
| @tag :tmp_dir | |||
| test("run the download", %{tmp_dir: tmp_dir}) do | |||
There was a problem hiding this comment.
I could not find a more suitable description for the test. Do you have any ideas?
There was a problem hiding this comment.
Consider proposal #150 , if we come to reach an angreement on that we have to fix that in another PR of course.
my suggestions to
Onigumo.Downloader.main:
- start
- init
- trigger
"run the download"
- activate downloading
- start download process
- integration test for downloader
- check download process
There was a problem hiding this comment.
We discussed personally, there is a option to split func main to init and run, where start should be located in init and rest in main....
There was a problem hiding this comment.
So far we keep the name main
There was a problem hiding this comment.
We kept main because the Downloader is essentialy a standalone application and the main function is its entry points. The caller does not care about what the module does. main is a traditional name from the ages of C and Java.
There was a problem hiding this comment.
The new name of the tests is „run Downloader,“ because what we test here is that the Downloader does its job. The fact that it loadts the URLs from a file and then downloads them is a mere technical detail.
|
|
Your changes are great, @nappex! Feel free to ✅ . |
The application
The new test is very similar to download_urls_from_file. In this case, the Stream runs automatically, and the test only verifies the result. The assertion of the Stream.run return value is redundant, but in another test case, we check that File.write! returns :ok, although there is no other possible return value.
The assertion of HTTPoison.start is mandatory: Mox requires all mock calls to have an expect clause. There is no need to test that separately then.
Fixes #56.