Test cleanup #422
Test cleanup #422
Conversation
| get api_v1_airtable_mentorships_path, headers: @headers, as: :json | ||
|
|
||
| assert response.status == 200 | ||
| assert_equal response.status, 200 |
There was a problem hiding this comment.
Is this out of order if format is:
assert_equal {expected}, {actual}
Does it matter if it's reversed?
There was a problem hiding this comment.
I may have phoned it in a bit at the end
| assert response.dig('fields', 'Skillsets') == [request_body[:skillsets]] | ||
| assert response.dig('fields', 'Additional Details') == request_body[:additional_details] | ||
| assert response.dig('fields', 'Mentor Requested') == [request_body[:mentor_requested]] | ||
| assert_equal request_body[:slack_user], response.dig('fields', 'Slack User') |
There was a problem hiding this comment.
this looks like {actual}, {expected}
There was a problem hiding this comment.
That is actually correct, the request_body is the hash of expected values.
| assert pr['number'] == @pr_number | ||
| assert pr['title'] == 'Revert "waffle.io Badge"' | ||
| assert pr['state'] == 'closed' | ||
| assert_equal 200, response.code.to_i |
There was a problem hiding this comment.
This appears to be
{expected}, {actual}
test/requests/git_hub/client_test.rb
Outdated
| assert response.headers['X-RateLimit-Limit'] == '60' | ||
| assert response.headers['X-RateLimit-Reset'] == '1503866476' | ||
| assert @client.send(:reset_time, response) == '8/27/2017 at 8:41:16pm UTC' | ||
| reset_time = Time.at(1503866476).strftime '%-m/%d/%Y at%l:%M:%S%P %Z' |
There was a problem hiding this comment.
How does this break test runs?
There was a problem hiding this comment.
When running locally it uses the local time zone when generating the timestamp which doesn’t match the expected value which is UTC.
There was a problem hiding this comment.
So I remember that sometimes the rate limiting test messes things up. We should probably have a way of testing rate limiting.
Why is this done instead of some other way?
There was a problem hiding this comment.
I can pull in something like timecop to manage the expected timezone
2 participants
Description of changes
Swaps out use of
assertwithassert_equalsand fixed a timezone issue when running tests locally.Based off #421