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.

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

ImageDownloaderTests reliability improvements #1361

Closed
1ec5 opened this issue Apr 30, 2018 · 5 comments
Closed

ImageDownloaderTests reliability improvements #1361

1ec5 opened this issue Apr 30, 2018 · 5 comments
Assignees
Labels
bug Something isn’t working build Issues related to builds and dependency management.

Comments

@1ec5
Copy link
Contributor

1ec5 commented Apr 30, 2018

testDownloadingImageAgainAfterFirstDownloadCompletes occasionally fails on Bitrise with an error reminiscent of #1327. Only Xcode 9.3 builds fail this way. For example, this build for #1343 failed:

testDownloadingImageAgainAfterFirstDownloadCompletes, XCTAssertTrue failed - Semaphore timed out

MapboxNavigationTests.ImageDownloaderTests
testDownloadingImageAgainAfterFirstDownloadCompletes, XCTAssertTrue failed - Semaphore timed out
/Users/vagrant/git/MapboxNavigationTests/ImageDownloaderTests.swift:120

      let secondSemaphoreResult = semaphore.wait(timeout: XCTestCase.NavigationTests.timeout)
      XCTAssert(secondSemaphoreResult == .success, "Semaphore timed out")
      
 Executed 47 tests, with 1 failure (0 unexpected) in 12.690 (12.786) seconds

/cc @akitchen

@1ec5 1ec5 added build Issues related to builds and dependency management. - tests labels Apr 30, 2018
@akitchen
Copy link
Contributor

@1ec5 Did this test run include the code changes in cb95112 ?

If this is indeed occurring with the URLProtocol registration changes in place, then we need to put a bit more instrumentation in place. It's also likely the case that we're not properly handling a URL loading sad path case (which can be reproduced by running the test without the URLProtocol registered).

@akitchen akitchen self-assigned this Apr 30, 2018
@1ec5
Copy link
Contributor Author

1ec5 commented Apr 30, 2018

Yes, that branch is based on 31848f3 and it has failed a few times now.

@akitchen
Copy link
Contributor

akitchen commented May 1, 2018

oof, that's frustrating. I have a few ideas as to what could be going on here, looking at other URLProtocol implementations.

Meanwhile I'll open a PR to disable the test.

@akitchen akitchen added the bug Something isn’t working label May 1, 2018
@akitchen
Copy link
Contributor

There were a few issues in ImageDownloaderTests and ImageDownloader itself which should be addressed in #1405

@akitchen akitchen changed the title testDownloadingImageAgainAfterFirstDownloadCompletes fails occasionally — Semaphore timed out ImageDownloaderTests reliability improvements May 14, 2018
@akitchen
Copy link
Contributor

The symptom described here was fixed in #1405

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn’t working build Issues related to builds and dependency management.
Projects
None yet
Development

No branches or pull requests

2 participants