Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add testPerformingCancelledRequest #52

Merged
merged 2 commits into from
Jan 28, 2016
Merged

Add testPerformingCancelledRequest #52

merged 2 commits into from
Jan 28, 2016

Conversation

8W9aG
Copy link
Contributor

@8W9aG 8W9aG commented Jan 27, 2016

  • Test cases where the request has been cancelled
    during authorization

@8W9aG
Copy link
Contributor Author

8W9aG commented Jan 27, 2016

@jgavris @rastersize @dflems @JensAyton

jgavris
jgavris reviewed Jan 27, 2016
View reviewed changes
SPTDataLoaderTests/SPTDataLoaderRequestResponseHandlerMock.m
@@ -44,4 +44,13 @@ - (void)receivedInitialResponse:(SPTDataLoaderResponse *)response
self.lastReceivedResponse = response;
}

- (BOOL)shouldAuthoriseRequest:(SPTDataLoaderRequest *)request
{
return self.authorising;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use the getter!

@8W9aG
Add testPerformingCancelledRequest
722d3cc 
* Test cases where the request has been cancelled
during authorization
@8W9aG 8W9aG force-pushed the add-testPerformingCancelledRequest branch from 8912fd7 to 722d3cc Compare January 27, 2016 18:58
@rastersize
Copy link
Contributor

The test fails for OS X and tvOS (same issue).

SPTDataLoaderServiceTest
  testPerformingCancelledRequest, ((self.service.handlers.count) equal to (0u)) failed: ("1") is not equal to ("0") - There should be no handlers for an already cancelled request
  /Users/travis/build/spotify/SPTDataLoader/SPTDataLoaderTests/SPTDataLoaderServiceTest.m:356

@8W9aG
Fix issue with not copying cancellationToken
f615cc5 
* Very good that the previous test showed that it
was an issue :)
* The cancellation token was not getting copied into
the new request made and thus was getting lost
making it become deallocated (since the request
only holds onto it weakly)
@dflems
Copy link
Contributor

dflems commented Jan 28, 2016

lgtm 👍

@rastersize
Copy link
Contributor

👍

8W9aG added a commit that referenced this pull request Jan 28, 2016
@8W9aG
Merge pull request #52 from 8W9aG/add-testPerformingCancelledRequest
29d7d26 
Add testPerformingCancelledRequest
@8W9aG 8W9aG merged commit 29d7d26 into spotify:master Jan 28, 2016
@8W9aG 8W9aG deleted the add-testPerformingCancelledRequest branch January 28, 2016 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
unit test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@8W9aG @rastersize @dflems @jgavris