-
Notifications
You must be signed in to change notification settings - Fork 808
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
Disable NSURLSession DownloadTaskWithURL_WithCancelResume test #1740
Conversation
[NSThread sleepForTimeInterval:0.5]; | ||
currentBytesDownloaded = (double)downloadTaskTestHelper.totalBytesWritten / 1024 / 1024; | ||
ASSERT_EQ_MSG(currentBytesDownloaded, lastBytesDownloaded, "FAILED: File download should have stopped!"); | ||
ASSERT_EQ(NSURLSessionTaskStateCanceling, downloadTask.state); |
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.
Can it still be "canceling but not cancelled" at this point?
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.
As I understand it Canceling is "Cancelled"
[NSThread sleepForTimeInterval:0.5]; | ||
lastBytesDownloaded = currentBytesDownloaded; | ||
currentBytesDownloaded = (double)downloadTaskTestHelper.totalBytesWritten / 1024 / 1024; | ||
double currentBytesDownloaded = (double)downloadTaskTestHelper.totalBytesWritten / 1024 / 1024; |
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.
.f
floating point arithmetic
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.
(float) / (integral) => float #WontFix
currentBytesDownloaded = (double)downloadTaskTestHelper.totalBytesWritten / 1024 / 1024; | ||
ASSERT_EQ_MSG(currentBytesDownloaded, lastBytesDownloaded, "FAILED: File download should have stopped!"); | ||
ASSERT_EQ(NSURLSessionTaskStateCanceling, downloadTask.state); | ||
lastBytesDownloaded = (double)downloadTaskTestHelper.totalBytesWritten / 1024 / 1024; |
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.
same here
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.
💯 cake
@@ -673,14 +670,12 @@ inline int _GetLastDelegateCall(NSURLSessionDownloadTaskTestHelper* downloadTask | |||
[conditionCancelled unlock]; | |||
|
|||
// Make sure download has stopped. | |||
lastBytesDownloaded = (double)downloadTaskTestHelper.totalBytesWritten / 1024 / 1024; | |||
[NSThread sleepForTimeInterval:0.5]; |
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.
PLEASE PLEASE PLEASE NO.
This is NOT a good way to test this sort of thing. These tests should honestly be scrapped and rewritten using events / mocks / something so we aren't guessing on when it will complete. It just slows down things for everyone and only adds minimal more reliability because they still hit real endpoints that could take arbitrary time to respond ....
🕐 |
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.
Sorry, switching my status to reject. Let's disable this test to unblock automated testing and revisit #1737.
It seems NSURLSession DownloadTaskWithURL_WithCancelResume is still finnicky. This disables the test until a better testing method is implemented.
This change is