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

MLIBZ-2050: LiveStreamAcl default constructor must be available #232

Merged
merged 2 commits into from Aug 26, 2017

Conversation

Projects
None yet
3 participants
@heyzooi
Contributor

heyzooi commented Aug 25, 2017

Description

LiveStreamAcl default constructor was internal, but should be public

Changes

  • Adding a explicit constructor for LiveStreamAcl

Tests

  • Unit test added to check if the constructor with no parameters is available

@heyzooi heyzooi self-assigned this Aug 25, 2017

@heyzooi heyzooi requested a review from tejasranade Aug 25, 2017

class RealtimeAclTestCase: KinveyTestCase {
func testLiveStreamAclConsturctorNoParams() {

This comment has been minimized.

@tejasranade

tejasranade Aug 25, 2017

Member

@heyzooi there's a typo in the method name :)
Also, it'll be nice to add some more tests around the other parameters in the constructor.

@tejasranade

tejasranade Aug 25, 2017

Member

@heyzooi there's a typo in the method name :)
Also, it'll be nice to add some more tests around the other parameters in the constructor.

This comment has been minimized.

@heyzooi

heyzooi Aug 25, 2017

Contributor

ok, I will add more tests specifically for ACL on LiveService

@heyzooi

heyzooi Aug 25, 2017

Contributor

ok, I will add more tests specifically for ACL on LiveService

This comment has been minimized.

@heyzooi

heyzooi Aug 26, 2017

Contributor

as we discussed, I will create new tickets to test ACL on LiveService and merge this PR as is now

@heyzooi

heyzooi Aug 26, 2017

Contributor

as we discussed, I will create new tickets to test ACL on LiveService and merge this PR as is now

func testLiveStreamAclConsturctorNoParams() {
let acl = LiveStreamAcl()
XCTAssertNotNil(acl)

This comment has been minimized.

@tejasranade

tejasranade Aug 25, 2017

Member

Also, when the constructor is used without parameters, what should be the default values of the properties of the class? Is it worth asserting those?
What about usage of this class? Is that tested anywhere?

@tejasranade

tejasranade Aug 25, 2017

Member

Also, when the constructor is used without parameters, what should be the default values of the properties of the class? Is it worth asserting those?
What about usage of this class? Is that tested anywhere?

This comment has been minimized.

@heyzooi

heyzooi Aug 25, 2017

Contributor

no, we don't have tests for ACL for Realtime yet, the only reason for this test is to make sure that a constructor without any parameters is available since it should be the most common use case. Our devcenter has an example that demonstrates this usage: https://devcenter.kinvey.com/ios/guides/live-service#DirectedCommunication

@heyzooi

heyzooi Aug 25, 2017

Contributor

no, we don't have tests for ACL for Realtime yet, the only reason for this test is to make sure that a constructor without any parameters is available since it should be the most common use case. Our devcenter has an example that demonstrates this usage: https://devcenter.kinvey.com/ios/guides/live-service#DirectedCommunication

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Aug 25, 2017

Codecov Report

Merging #232 into develop will decrease coverage by 0.01%.
The diff coverage is 14.28%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #232      +/-   ##
===========================================
- Coverage    90.83%   90.81%   -0.02%     
===========================================
  Files           65       65              
  Lines         7681     7777      +96     
===========================================
+ Hits          6977     7063      +86     
- Misses         704      714      +10
Flag Coverage Δ
#Mac 88.05% <14.28%> (-0.04%) ⬇️
#iOS 89.69% <14.28%> (-0.01%) ⬇️
Impacted Files Coverage Δ
Kinvey/Kinvey/Realtime.swift 73.13% <14.28%> (-2.13%) ⬇️
Kinvey/Kinvey/RealmCache.swift 87.36% <0%> (+0.58%) ⬆️
Kinvey/Kinvey/Kinvey.swift 100% <0%> (+0.9%) ⬆️
Kinvey/Kinvey/Client.swift 92.61% <0%> (+0.98%) ⬆️
Kinvey/Kinvey/Persistable.swift 83.24% <0%> (+1.45%) ⬆️

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 6c4d8c4...50b64a5. Read the comment docs.

codecov-io commented Aug 25, 2017

Codecov Report

Merging #232 into develop will decrease coverage by 0.01%.
The diff coverage is 14.28%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #232      +/-   ##
===========================================
- Coverage    90.83%   90.81%   -0.02%     
===========================================
  Files           65       65              
  Lines         7681     7777      +96     
===========================================
+ Hits          6977     7063      +86     
- Misses         704      714      +10
Flag Coverage Δ
#Mac 88.05% <14.28%> (-0.04%) ⬇️
#iOS 89.69% <14.28%> (-0.01%) ⬇️
Impacted Files Coverage Δ
Kinvey/Kinvey/Realtime.swift 73.13% <14.28%> (-2.13%) ⬇️
Kinvey/Kinvey/RealmCache.swift 87.36% <0%> (+0.58%) ⬆️
Kinvey/Kinvey/Kinvey.swift 100% <0%> (+0.9%) ⬆️
Kinvey/Kinvey/Client.swift 92.61% <0%> (+0.98%) ⬆️
Kinvey/Kinvey/Persistable.swift 83.24% <0%> (+1.45%) ⬆️

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 6c4d8c4...50b64a5. Read the comment docs.

@heyzooi heyzooi merged commit d795ddf into develop Aug 26, 2017

3 checks passed

codebeat no reportable quality changes
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@heyzooi heyzooi deleted the feature/MLIBZ-2050-default_constructor_livestreamacl branch Aug 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment