-
Notifications
You must be signed in to change notification settings - Fork 1
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
Parametrize output filename #40
Conversation
This allows to use `tmp_dir` for `download/2` (now `/3`) tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized we may have picked a suboptimal argument order for the parametrized filename. What do you think, @nappex?
end | ||
|
||
def download(url, http) do | ||
def download(url, http, path) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just thinking, wouldn’t it be more intuitive to have the literal input data collected on one side and the behaviors on the other side? Considering we already moved url
to the beginning, I’d bring path
there too.
def download(url, http, path) do | |
def download(url, path, http) do |
@@ -10,16 +10,16 @@ defmodule Onigumo do | |||
http = http_client() | |||
|
|||
load_urls(@input_path) | |||
|> Enum.map(&download(&1, http)) | |||
|> Enum.map(&download(&1, http, @output_path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|> Enum.map(&download(&1, http, @output_path)) | |
|> Enum.map(&download(&1, @output_path, http)) |
@@ -20,7 +21,7 @@ defmodule OnigumoTest do | |||
end | |||
) | |||
|
|||
assert(:ok == Onigumo.download(@url, HTTPoisonMock)) | |||
assert(:ok == Onigumo.download(@url, HTTPoisonMock, @output_path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert(:ok == Onigumo.download(@url, HTTPoisonMock, @output_path)) | |
assert(:ok == Onigumo.download(@url, @output_path, HTTPoisonMock)) |
This allows to use
tmp_dir
fordownload/2
(now/3
) tests.Fixes #32 if not considering #29.