Scribe-Android icon indicating copy to clipboard operation
Scribe-Android copied to clipboard

Feat/write test fns for about util

Open DeleMike opened this issue 6 months ago • 9 comments
trafficstars

Contributor checklist


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

DeleMike avatar May 05 '25 15:05 DeleMike

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 😊

github-actions[bot] avatar May 05 '25 15:05 github-actions[bot]

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 :)

  • [ ] The linting and formatting workflows within the PR checks do not indicate new errors in the files changed

  • [ ] The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

github-actions[bot] avatar May 05 '25 15:05 github-actions[bot]

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

DeleMike avatar May 05 '25 15:05 DeleMike

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?

DeleMike avatar May 05 '25 15:05 DeleMike

I will look into this PR tmrw evening.

angrezichatterbox avatar May 05 '25 17:05 angrezichatterbox

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.

angrezichatterbox avatar May 06 '25 12:05 angrezichatterbox

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?

DeleMike avatar May 06 '25 12:05 DeleMike

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.

angrezichatterbox avatar May 06 '25 12:05 angrezichatterbox

okay, I'll look into it and share my feedback if any

DeleMike avatar May 06 '25 13:05 DeleMike

Hi @angrezichatterbox , here is an update on the testing issue.

I have added new test cases and resolved the failing tests.

cc: @andrewtavis

DeleMike avatar Jun 09 '25 13:06 DeleMike

Thanks for the great work, @DeleMike! @angrezichatterbox, let us know if anything further is needed here 😊

andrewtavis avatar Jun 09 '25 13:06 andrewtavis

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?

DeleMike avatar Jun 10 '25 21:06 DeleMike

Ideally we'd also see failing tests in the PR jobs, @angrezichatterbox :) Is there a reason why the tests are passing?

andrewtavis avatar Jun 10 '25 21:06 andrewtavis

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.

angrezichatterbox avatar Jun 11 '25 03:06 angrezichatterbox

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.

angrezichatterbox avatar Jun 11 '25 03:06 angrezichatterbox

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 😊

DeleMike avatar Jun 11 '25 07:06 DeleMike

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 :)

andrewtavis avatar Jun 11 '25 08:06 andrewtavis

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 :)

@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

DeleMike avatar Jun 11 '25 08:06 DeleMike

#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!

andrewtavis avatar Jun 21 '25 20:06 andrewtavis

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

DeleMike avatar Jun 22 '25 04:06 DeleMike

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.

Screenshot 2025-06-22 at 17 05 31

DeleMike avatar Jun 22 '25 16:06 DeleMike

Nice that the tests are passing now, @DeleMike! Please let us know when this is ready for a review 😊

andrewtavis avatar Jun 22 '25 19:06 andrewtavis

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):

  1. AndroidRuntimeException – startActivity() from non-Activity context Issue: The ShareHelper.shareScribe() and ShareHelper.sendEmail() functions were using ContextCompat.startActivity() with a non-Activity context (e.g., Application context from InstrumentationRegistry), which throws unless FLAG_ACTIVITY_NEW_TASK is 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)
    }
  1. NoActivityResumedException in tests like test_onShareScribeClick_doesNotCrash
  • Issue: The test directly used application context instead of launching an actual Activity.

  • Fix: I wrapped AboutUtil.onShareScribeClick() inside an ActivityScenario.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.

DeleMike avatar Jun 22 '25 19:06 DeleMike