Fixing some tests. #370
Fixing some tests. #370
Conversation
Some tests are failing because https://ca.ios.ba doesn't have HTTPS enabled at the moment. In any case, it's better to rely in another URL that the team has more control over, so I replaced all instances by meedan.com.
|
Code Climate has analyzed commit d3742cb and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 98.0% (0.0% change). View more on Code Climate. |
| url = 'https://ca.ios.ba/files/others/test.php' # This link has a sleep(10) function | ||
| timeout = '2' | ||
| api_key = create_api_key application_settings: { config: { timeout: timeout }} | ||
| url = 'https://meedan.com/' |
There was a problem hiding this comment.
there was a sleep function in the link before, that isn't needed here?
There was a problem hiding this comment.
No, I reduced the config.timeout to compensate.
| @lw = Lapis::Webhook.new('https://ca.ios.ba/', { foo: 'bar' }.to_json) | ||
| super | ||
| url = 'https://webhook.net/' | ||
| @lw = Lapis::Webhook.new(url, { foo: 'bar' }.to_json) |
There was a problem hiding this comment.
here super isn't needed?
There was a problem hiding this comment.
Thanks for catching, but no, it was intended... super was not needed for that suite, so it was just making things slightly slower and could lead to surprises too.
| assert_equal 'Timeout', JSON.parse(@response.body)['data']['error']['message'] | ||
| assert_response 200 | ||
| end | ||
|
|
||
| test "should return success even if media could not be instantiated" do | ||
| authenticate_with_token | ||
| Media.expects(:new).raises(Net::ReadTimeout) | ||
| get :index, params: { url: 'http://ca.ios.ba/files/meedan/random.php', format: :json, refresh: '1' } | ||
| get :index, params: { url: 'https://meedan.com', format: :json, refresh: '1' } |
There was a problem hiding this comment.
here the random.php isn't needed as well? why?
There was a problem hiding this comment.
The URL in that case could be any, because the method is mocked anyway: Media.expects(:new).raises(Net::ReadTimeout).
|
Thanks @vasconsaurus for the thoroughly review! :) |
Description
Some tests are failing because https://ca.ios.ba doesn't have HTTPS enabled at the moment. In any case, it's better to rely in another URL that the team has more control over, so I replaced all instances by meedan.com.
How has this been tested?
The build should be fixed with these changes.
Checklist