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

Set appropriate protection levels on NetworkReachabilityManager members #2393

Merged
merged 1 commit into from Feb 18, 2018

Conversation

Projects
None yet
2 participants
@parski
Contributor

parski commented Dec 21, 2017

Goals ⚽️

I want to be able to mock NetworkReachabilityManager for unit testing in integrating projects. The current protection level of public does not allow inheritance forcing me to inconvenient testing methods with wrappers and the like. By changing the protection level to open the class and the appropriate members can be subclassed and overridden making mocking it is a breeze.

Implementation Details 🚧

I've changed protection levels from public to open where applicable.

Testing Details 🔍

I used a test project to reproduce the problem with inheritance and subsequent overriding. I made my changes and ran the test suite to make sure everything passes. I was thinking of creating a mock of NetworkReachabilityManager and adding it to the test target of the example app but I'm not sure it's a viable means of testing since compilation would be the assertion and the mock would be unused. Let me know if you think any kind of test is appropriate for this PR.

@jshier jshier added this to the 4.7.0 milestone Feb 18, 2018

@jshier

This comment has been minimized.

Contributor

jshier commented Feb 18, 2018

Looks good, thanks! This should go out with the 4.7.0 release soon.

@jshier jshier merged commit fa2645b into Alamofire:master Feb 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment