Scribe-Android
Scribe-Android copied to clipboard
Feat/write test fns for about util
Contributor checklist
- [x] This pull request is on a separate branch and not the main branch
- [x] I have tested my code with the
./gradlew lintKotlin detekt testcommand as directed in the testing section of the contributing guide
Description
This PR contains tests for the AboutUtil.kt.
Added tests for the following AboutUtil functions:
-
Non-UI functions:
- onShareScribeClick
- onMailClick
-
Composable functions:
- getCommunityList (covers all items and their click behavior)
- getFeedbackAndSupportList (covers callback and intent-triggered items)
- getLegalListItems
Note: onRateScribeClick test is pending. I don't know how to actually implement it. Or what to test...
Also Note: I removed dependencies relating to JUnit5. It was causing some conflicts when I tried to build/run the tests. I initially started writing with JUnit5 but I had to remove it because existing codebase already uses JUnit4 and I had an issue with Compose's compatibility with JUnit5
Related issue
- closes #358
Thank you for the pull request! ❤️
The Scribe-Android team will do our best to address your contribution as soon as we can. If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Android rooms once you're in. Also consider attending our bi-weekly Saturday dev syncs. It'd be great to meet you 😊
Maintainer Checklist
The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)
Hi @angrezichatterbox 👋🏾, can you check out this PR? I have some issues writing the onRateScribeClick test function. I'm hoping you can help shed more light. However, overall, what do you think?
cc: @andrewtavis
fixed the license issue :) but I see this as a reason why the other two fail:
AboutUtilTest > testOnMailClick_sendsEmailIntent FAILED
java.lang.RuntimeException at RoboMonitoringInstrumentation.java:102
AboutUtilTest > testGetCommunityListTriggersCallbacks FAILED
java.lang.RuntimeException at RoboMonitoringInstrumentation.java:102
AboutUtilTest > testGetFeedbackAndSupportListTriggersCallbacks FAILED
java.lang.RuntimeException at RoboMonitoringInstrumentation.java:102
AboutUtilTest > testOnShareScribeClick FAILED
java.lang.RuntimeException at RoboMonitoringInstrumentation.java:102
AboutUtilTest > testGetLegalListItemsTriggersCallbacks FAILED
java.lang.RuntimeException at RoboMonitoringInstrumentation.java:102
can you explain why this is happening? Instrumentation?
I will look into this PR tmrw evening.
Hey, We decided on using Mockk for the project as it offered more advantages to the project in the long term due to the Mockk being more adjusted to use Kotlin rather than Mockito as it is more adjusted to use Java.
Could the tests be migrated to use Mockk.
Hey, We decided on using Mockk for the project as it offered more advantages to the project in the long term due to the Mockk being more adjusted to use Kotlin rather than Mockito as it is more adjusted to use Java.
Could the tests be migrated to use Mockk.
Yes, it can. I'll try to do that.
is that the reason why the teo tests are failing?
It would be better to write instrumentation tests where Roboelectric has to be necessary used. I would recommend writing instrumentation tests for some of these rather than Unit tests as these deals with interaction with the device as well.
okay, I'll look into it and share my feedback if any
Hi @angrezichatterbox , here is an update on the testing issue.
I have added new test cases and resolved the failing tests.
cc: @andrewtavis
Thanks for the great work, @DeleMike! @angrezichatterbox, let us know if anything further is needed here 😊
Thank you @angrezichatterbox. For the reports, as discussed here, do you want me to paste the reports output here?
And for the tests, I tried it again and they are all passing. Can you highlight the two tests that are failing?
Ideally we'd also see failing tests in the PR jobs, @angrezichatterbox :) Is there a reason why the tests are passing?
Thank you @angrezichatterbox. For the reports, as discussed here, do you want me to paste the reports output here?
And for the tests, I tried it again and they are all passing. Can you highlight the two tests that are failing?
I'll get back to you on this later today.
Ideally we'd also see failing tests in the PR jobs, @angrezichatterbox :) Is there a reason why the tests are passing?
We haven't set it up for git actions for Instrumentation tests yet.
Thank you @angrezichatterbox. For the reports, as discussed here, do you want me to paste the reports output here? And for the tests, I tried it again and they are all passing. Can you highlight the two tests that are failing?
I'll get back to you on this later today.
Alright, I'll be waiting for a reply 😊
Thanks for letting me know about the action, @angrezichatterbox! @DeleMike, can you make an issue to add the instrumentation tests to the GitHub actions? Including the command to run would be great, and we could do help wanted and good first issue for it 😊 You'd be welcome to work on it yourself if you have interest, but it might also be a great candidate for the hackathon in July :)
Thanks for letting me know about the action, @angrezichatterbox! @DeleMike, can you make an issue to add the instrumentation tests to the GitHub actions? Including the command to run would be great, and we could do
help wantedandgood first issuefor it 😊 You'd be welcome to work on it yourself if you have interest, but it might also be a great candidate for the hackathon in July :)
@andrewtavis Yes, I can make an issue. Yes, I could work on that so that we bring this PR quickly.
I will create the issue and I will tag you and @angrezichatterbox
#411 has been merged in and we're ready to work on other PRs at this point, @DeleMike :) Please let us know if you need support with the merge conflicts here!
Hi @andrewtavis , I will work on this PR today.
The main issue left here is, we need to wait for the Emulator to finish booting and I've been researching on how to make this possible. But I'll report my current status soon!
I will resolve merge conflicts and work on it
I have solved the merge conflicts, and the Android tests are passing on my device, but for some reason, the AndroidTests Github actions flow keeps failing. @andrewtavis @angrezichatterbox
I will investigate further.
Nice that the tests are passing now, @DeleMike! Please let us know when this is ready for a review 😊
Hello @angrezichatterbox and @andrewtavis. The PR is ready for review.
🛠 Summary of Fixes for Failing Instrumented Tests
I resolved two core issues affecting the tests (because they were passing on physical devices and not emulators):
AndroidRuntimeException – startActivity()from non-Activity context Issue: TheShareHelper.shareScribe()andShareHelper.sendEmail()functions were usingContextCompat.startActivity()with a non-Activity context (e.g., Application context from InstrumentationRegistry), which throws unlessFLAG_ACTIVITY_NEW_TASKis added.
Fix: I added a runtime check to append Intent.FLAG_ACTIVITY_NEW_TASK if the provided context is not an Activity:
if (context !is Activity) {
addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
}
NoActivityResumedExceptionin tests liketest_onShareScribeClick_doesNotCrash
-
Issue: The test directly used application context instead of launching an actual Activity.
-
Fix: I wrapped
AboutUtil.onShareScribeClick()inside anActivityScenario.launch()so that there's an active Activity:
val scenario = ActivityScenario.launch(MainActivity::class.java)
scenario.onActivity { activity ->
AboutUtil.onShareScribeClick(activity)
}
✅ All instrumentation tests now pass on the emulator started by our GitHub actions test and on physical devices.
Please let me know if you need anything else.