vadymmarkov / Malibu

:surfer: Malibu is a networking library built on promises

https://vadymmarkov.github.io

Other

414 stars 39 forks source link

Make URLSession injectable into `Networking` for easy mocking. #115

Open codeOfRobin opened 5 years ago

codeOfRobin commented 5 years ago

Signed-off-by: Robin Malhotra me@rmalhotra.com

codecov-io commented 5 years ago

Codecov Report

Merging #115 into master will increase coverage by 0.02%. The diff coverage is 75%.

@@            Coverage Diff             @@
##           master     #115      +/-   ##
==========================================
+ Coverage   92.73%   92.76%   +0.02%     
==========================================
  Files          64       64              
  Lines        3110     3108       -2     
==========================================
- Hits         2884     2883       -1     
+ Misses        226      225       -1

Impacted Files	Coverage Δ
Sources/Networking.swift	`58.52% <75%> (+0.09%)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3b9a852...234a5e7. Read the comment docs.

vadymmarkov commented 5 years ago

I think it would be better to keep both init methods, one with session configuration and one with session. What do you think?

codeOfRobin commented 5 years ago

That works too! Have the init with sessionConfiguration generate a URLSession in the init. Would you like me to add that to the PR?

vadymmarkov commented 5 years ago

Yes, please 😉

codeOfRobin commented 5 years ago

One problem now 😕, since both declarations look identical to the compiler,

the empty initializer confuses the compiler 😞

Any way you can think of to get around this? (also, need to fix that vertical_alignment thing that won't go away :/

vadymmarkov commented 5 years ago

It should work if you drop default parameter for session: URLSession and create it like Networking(session: .shared) when needed.

codeOfRobin commented 5 years ago

Done 😄

vadymmarkov commented 5 years ago

Great! Any idea why tests are failing?

codeOfRobin commented 5 years ago

@vadymmarkov I just tried running those tests locally. The test that failed on CI (NetworkingSpec.Networking___urlSession_didReceiveChallenge_completionHandler___with_NSURLAuthenticationMethodServerTrust__without_baseUrl__passess_valid_parameters_to_completion() passed over here :/

Could you try running it locally on your computer to see if you can reproduce it?

codeOfRobin commented 5 years ago

Wait a second, I noticed the tests are running on Xcode 9.4. Could that be a cause?

codeOfRobin commented 5 years ago

Got a branch with an Xcode 10.1 image in the .travis.yml: https://github.com/codeOfRobin/Malibu/tree/travis-new-image

codeOfRobin commented 5 years ago

Hi 👋 @vadymmarkov gentle nudge, was hoping you could take a look at this 😬