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

Fix library instrumented tests #27

Merged

Conversation

tatocaster
Copy link
Contributor

@tatocaster tatocaster commented Jan 4, 2022

  • Fixed test in ContactStoreDSLKtTest.kt, matched fields
  • Edited assertContactUpdated - compare object contents

Copy link
Owner

@alexstyl alexstyl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution again @tatocaster. It's a bit embarasing to have tests failing as I probably forgot to run the tests before pushing some changes it seems.

I am currently not on my laptop and I can properly check the PR later in the day (if not tomorrow morning), as I need to see what exactly is failing to say if this is the right solution or not.

It would help if you could update the PR with the log of what is failing. It's totally fine if it is too much hussle.

@tatocaster
Copy link
Contributor Author

This is the output from ContactStoreDSLKtTest.kt

Expected :SaveRequest(_requests=[Insert(contact=MutableContact(contactId=-1, displayName='pref Paolo Mid Melendez, Suf' lookupKey=null, isStarred=false, columns=[com.alexstyl.contactstore.ContactColumn$Phones@af765e6, com.alexstyl.contactstore.ContactColumn$Mails@18ec2d30, com.alexstyl.contactstore.ContactColumn$Note@38c3ba52, com.alexstyl.contactstore.ContactColumn$Events@16291182, com.alexstyl.contactstore.ContactColumn$PostalAddresses@27c09ef4, com.alexstyl.contactstore.ContactColumn$Image@5e3cb291, com.alexstyl.contactstore.ContactColumn$Names@7b88822e, com.alexstyl.contactstore.ContactColumn$Nickname@257339dc, com.alexstyl.contactstore.ContactColumn$WebAddresses@66724b8, com.alexstyl.contactstore.ContactColumn$Organization@3d74a6df, com.alexstyl.contactstore.ContactColumn$GroupMemberships@252caad, com.alexstyl.contactstore.ContactColumn$ImAddresses@3ed3bee2, com.alexstyl.contactstore.ContactColumn$Relations@12e0f64, com.alexstyl.contactstore.ContactColumn$SipAddresses@19f413c0], imageData=ImageData(raw=[105, 109, 97, 103, 101, 32, 100, 97, 116, 97]), phones=[LabeledValue(value=PhoneNumber(raw=555-11-23), label=com.alexstyl.contactstore.Label$LocationHome@2b18ee27, id=null)], mails=[LabeledValue(value=MailAddress(raw=paolo@paolo.com), label=com.alexstyl.contactstore.Label$LocationWork@64654fd5, id=null)], events=[LabeledValue(value=EventDate(dayOfMonth=3, month=12, year=2021), label=com.alexstyl.contactstore.Label$DateBirthday@4c53c497, id=null)], postalAddresses=[LabeledValue(value=PostalAddress(street=85 Somewhere Str, poBox=, neighborhood=, city=, region=, postCode=, country=), label=com.alexstyl.contactstore.Label$LocationHome@2b18ee27, id=null)], webAddresses=[LabeledValue(value=WebAddress(raw=www.paolo.com), label=com.alexstyl.contactstore.Label$LocationWork@64654fd5, id=null)], imAddresses=[], sipAddresses=[], linkedAccountValues=N/A, note=Note(raw=Note to self), groups=[GroupMembership(groupId=123, _id=null)], relations=[], organization=Org, jobTitle=Job Title, firstName=Paolo, lastName=Melendez, middleName=Mid, prefix=pref, suffix=Suf, phoneticLastName=null, phoneticFirstName=null, phoneticMiddleName=null, fullNameStyle=0, phoneticNameStyle=0, nickname=null))])
Actual   :SaveRequest(_requests=[Insert(contact=MutableContact(contactId=-1, displayName='pref Paolo Mid Melendez, Suf' lookupKey=null, isStarred=false, columns=[com.alexstyl.contactstore.ContactColumn$Phones@af765e6, com.alexstyl.contactstore.ContactColumn$Mails@18ec2d30, com.alexstyl.contactstore.ContactColumn$Note@38c3ba52, com.alexstyl.contactstore.ContactColumn$Events@16291182, com.alexstyl.contactstore.ContactColumn$PostalAddresses@27c09ef4, com.alexstyl.contactstore.ContactColumn$Image@5e3cb291, com.alexstyl.contactstore.ContactColumn$Names@7b88822e, com.alexstyl.contactstore.ContactColumn$Nickname@257339dc, com.alexstyl.contactstore.ContactColumn$WebAddresses@66724b8, com.alexstyl.contactstore.ContactColumn$Organization@3d74a6df, com.alexstyl.contactstore.ContactColumn$GroupMemberships@252caad, com.alexstyl.contactstore.ContactColumn$ImAddresses@3ed3bee2, com.alexstyl.contactstore.ContactColumn$Relations@12e0f64, com.alexstyl.contactstore.ContactColumn$SipAddresses@19f413c0], imageData=ImageData(raw=[105, 109, 97, 103, 101, 32, 100, 97, 116, 97]), phones=[LabeledValue(value=PhoneNumber(raw=555-11-23), label=com.alexstyl.contactstore.Label$LocationHome@2b18ee27, id=null)], mails=[LabeledValue(value=MailAddress(raw=paolo@paolo.com), label=com.alexstyl.contactstore.Label$LocationWork@64654fd5, id=null)], events=[LabeledValue(value=EventDate(dayOfMonth=3, month=12, year=2021), label=com.alexstyl.contactstore.Label$DateBirthday@4c53c497, id=null)], postalAddresses=[LabeledValue(value=PostalAddress(street=85 Somewhere Str, poBox=, neighborhood=, city=, region=, postCode=, country=), label=com.alexstyl.contactstore.Label$LocationHome@2b18ee27, id=null)], webAddresses=[LabeledValue(value=WebAddress(raw=www.paolo.com), label=com.alexstyl.contactstore.Label$LocationWork@64654fd5, id=null)], imAddresses=[], sipAddresses=[], linkedAccountValues=N/A, note=Note(raw=Note to self), groups=[GroupMembership(groupId=123, _id=null)], relations=[], organization=Org, jobTitle=Job Title, firstName=Paolo, lastName=Melendez, middleName=Mid, prefix=pref, suffix=Suf, phoneticLastName=, phoneticFirstName=, phoneticMiddleName=, fullNameStyle=0, phoneticNameStyle=0, nickname=))])
<Click to see difference>

org.junit.ComparisonFailure: expected:<...f, phoneticLastName=[null, phoneticFirstName=null, phoneticMiddleName=null, fullNameStyle=0, phoneticNameStyle=0, nickname=null]))])> but was:<...f, phoneticLastName=[, phoneticFirstName=, phoneticMiddleName=, fullNameStyle=0, phoneticNameStyle=0, nickname=]))])>
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at com.alexstyl.contactstore.ContactStoreDSLKtTest$insert$1.invokeSuspend(ContactStoreDSLKtTest.kt:55)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:274)
	at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:85)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:59)
	at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:38)
	at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source)
	at com.alexstyl.contactstore.ContactStoreDSLKtTest.insert(ContactStoreDSLKtTest.kt:12)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:110)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
	at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:38)
	at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:62)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:51)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at com.sun.proxy.$Proxy2.processTestClass(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.processTestClass(TestWorker.java:121)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
	at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
	at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
	at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:56)
	at java.base/java.lang.Thread.run(Thread.java:829)

@tatocaster
Copy link
Contributor Author

This is the output from AddValuesToExistingContactContactStoreTest.kt.
But fails for all the tests in UpdatesValuesOfExistingContactContactStoreTest.kt because they all depend on assertContactUpdated function

java.lang.AssertionError: 
Expected: <MutableContact(contactId=762, displayName='A. Paolo M. Melendez, Z.' lookupKey=null, isStarred=false, columns=[com.alexstyl.contactstore.ContactColumn$Names@ceb2f63], imageData=N/A, phones=N/A, mails=N/A, events=N/A, postalAddresses=N/A, webAddresses=N/A, imAddresses=N/A, sipAddresses=N/A, linkedAccountValues=N/A, note=N/A, groups=N/A, relations=N/A, organization=N/A, jobTitle=N/A, firstName=Paolo, lastName=Melendez, middleName=M., prefix=A., suffix=Z., phoneticLastName=, phoneticFirstName=, phoneticMiddleName=, fullNameStyle=0, phoneticNameStyle=0, nickname=N/A)>
     but: was <PartialContact(contactId=762, displayName='A. Paolo M. Melendez, Z.' lookupKey=LookupKey(value=0r762-294729453F454141313F31432F315B5B), isStarred=false, columns=[com.alexstyl.contactstore.ContactColumn$Names@ceb2f63], imageData=N/A, phones=N/A, mails=N/A, events=N/A, postalAddresses=N/A, webAddresses=N/A, imAddresses=N/A, sipAddresses=N/A, linkedAccountValues=N/A, note=N/A, groups=N/A, relations=N/A, organization=N/A, jobTitle=N/A, firstName=Paolo, lastName=Melendez, middleName=M., prefix=A., suffix=Z., phoneticLastName=, phoneticFirstName=, phoneticMiddleName=, fullNameStyle=1, phoneticNameStyle=0, nickname=N/A)>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:8)
	at com.alexstyl.contactstore.ContactStoreTestBase.assertContactUpdated(ContactStoreTestBase.kt:30)
	at com.alexstyl.contactstore.ContactStoreTestBase$assertContactUpdated$1.invokeSuspend(Unknown Source:15)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:274)
	at kotlinx.coroutines.BlockingCoroutine.joinBlocking(Builders.kt:85)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking(Builders.kt:59)
	at kotlinx.coroutines.BuildersKt.runBlocking(Unknown Source:1)
	at kotlinx.coroutines.BuildersKt__BuildersKt.runBlocking$default(Builders.kt:38)
	at kotlinx.coroutines.BuildersKt.runBlocking$default(Unknown Source:1)
	at com.alexstyl.contactstore.AddValuesToExistingContactContactStoreTest.updatesNames(AddValuesToExistingContactContactStoreTest.kt:23)

@tatocaster
Copy link
Contributor Author

The difference between objects are: lookupKey is null for the actual contact and fullNameStyle is 0 instead of 1.
mutableCopy fixes that because it modifies some of the properties

@alexstyl
Copy link
Owner

alexstyl commented Jan 5, 2022

The difference between objects are: lookupKey is null for the actual contact and fullNameStyle is 0 instead of 1.
mutableCopy fixes that because it modifies some of the properties

@tatocaster Seems like that is the cause of the failure. The Contact.mutableCopy() does not keep the lookupKey and the fullNameStyle is using the phoneticNameStyle instead of the fullNameStyle of the original contact.

PS: I had a quick look on the web and noticed that the 'lookupKey' is not as permanent as the documentation says. Could you update assertContactUpdated to check the contents of the actual instead of equality? Otherwise it seems like the test will fail because of lookupkey changes. There is a matcher called equalContents() that you can use in this case?

@alexstyl
Copy link
Owner

alexstyl commented Jan 5, 2022

Regarding the ContactStoreDSLKtTest I noticed that the MutableContactBuilder's default values do not match with MutableContact's default values (i.e. MutableContact's phoneticFirstName defaults to null, but Builder's one defaults to an empty string). Can you make sure that the Builder one matches the default values of the MutableContact one?

@tatocaster
Copy link
Contributor Author

fullNameStyle is using the phoneticNameStyle instead of the fullNameStyle of the original contact.

Should we leave it like that or?

Regarding the ContactStoreDSLKtTest I noticed that the MutableContactBuilder's default values do not match with MutableContact's default values (i.e. MutableContact's phoneticFirstName defaults to null, but Builder's one defaults to an empty string).

Yeah, I noticed them too. Thought it was part of the design. I was thinking the same and I wanted your review. Will update asap, thanks

@alexstyl
Copy link
Owner

alexstyl commented Jan 5, 2022

Should we leave it like that or?

Let's change it so that the new MutableContact takes the fullnameStyle of the previous one. It was probably a copy paste mistake. :)

@alexstyl
Copy link
Owner

alexstyl commented Jan 5, 2022

@tatocaster heads up: I have Ignored the failing tests on main.

@tatocaster
Copy link
Contributor Author

All tests are passing! ✅
I'd like to add Github Actions for the future PRs to run all the tests automatically. The idea for another contribution 😃

@alexstyl
Copy link
Owner

alexstyl commented Jan 5, 2022

Awesome. Seems 💯

Could you please squash your first 3 commits? I do not think there is a need to have those in git history as the real fix is the commits you pushed today.

After that I will merge to main 🚀

PS: Having the tests run via Github Actions would be amazing! I haven't used Actions at all, so I would be really keen to see how it works

use `equalContents` custom matcher to compare contact objects

Fix DSL test
- add fields to remove null from the result

Fix instrumented tests
convert PartialContact to MutableContact to pass the object equality check
@tatocaster tatocaster force-pushed the bugfix/fix_library_instrumented_tests branch from 6cfeef8 to 5320501 Compare January 5, 2022 23:24
@tatocaster tatocaster force-pushed the bugfix/fix_library_instrumented_tests branch from 8d63581 to 5bbd00b Compare January 5, 2022 23:29
@tatocaster
Copy link
Contributor Author

@alexstyl Done!
Android studio optimized imports, if we are OK with that then it's ready to merge. Otherwise, I can introduce Detekt or Ktlint to the project to avoid such changes.
Let me know what you think

@alexstyl
Copy link
Owner

alexstyl commented Jan 6, 2022

@tatocaster That is alright for now. The scope of the PR was to fix the tests and you did. 👏

The imports and coding style is something for an other PR. I will be creating some Github issues today for things to be done. If you are looking for more contributions until then, I think that having unit and Android tests running on PRs would be the most valuable for now.

Merging this to main. Once again thanks a ton for the contribution 💯

@alexstyl alexstyl merged commit d579152 into alexstyl:main Jan 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants