joreilly / Confetti

KMP GraphQL based conference project with Jetpack Compose Android, Compose for Wear and SwiftUI iOS clients along with GraphQL backend.
Apache License 2.0
720 stars 85 forks source link

Make RNG test classes accessible for Android App #522

Open oas004 opened 1 year ago

oas004 commented 1 year ago

I want to make Robolectric tests for the Android app for different configurations.

For now I think it is only wear module that has the RNG test classes. I'm wondering here if it makes sense to pull some of that out to a shared module? Specially the diffing code that is used with the quickBird library.

Do you have any opinion on this @yschimke since you wrote this for wear ? :)

yschimke commented 1 year ago

If you give me a day or two, I'd like to try some classes I defined in Horologist 0.4.0. not sure they will work well for mobile but a starting point.

oas004 commented 1 year ago

Alright, no worries! :) Looking forward to checking it out! :)

yschimke commented 1 year ago

Actually. Let's work on them here. I just realised there are way too many Wear dependencies in the ones I have.

oas004 commented 1 year ago

Alright, can I move the Snapshot class out to a common module maybe? And abstract away what is needed for wear? :)

yschimke commented 1 year ago

Yep. I also don't love the API. It it works better as a Rule, we should do that.

oas004 commented 1 year ago

Yeah maybe we can change that up? :) But does it make sense to maybe do that in two processes? My initial thought for this was to first move the Screenshot class stuff to :shared (androidMain source set) and the diffing maybe to the common JVM source set. Then the second process would be to change the API? WDYT?

yschimke commented 1 year ago

Whatever works for you.

oas004 commented 1 year ago

@yschimke Was there something you had to enable from the kotlin-snapshot-testing-library side to get the FileSnapshotting<Value, Format>.snapshot actually record a snapshot? I'm kinda rubber ducking a bit and for me the test always passes, but I can't get it to actually save a bitmap somewhere πŸ˜…

yschimke commented 1 year ago

There is a record flag you pass in. My test base class has a public mutable field for it.

But it should fail if reference file doesn't exist.

Also fails when record = true, so you don't check it in.

oas004 commented 1 year ago

Yeah I tried to copy that logic into the test rule in https://github.com/joreilly/Confetti/pull/528 but it just passes either way if I pass false or true πŸ€”

yschimke commented 1 year ago

I'll try later, see why.

oas004 commented 1 year ago

Thanks! :)

yschimke commented 1 year ago

So even just adding a TODO() into the start of the test isn't running and failing.

If I comment out the rule, it fails with a Firebase error, which maybe means removing this line? Only do this if you hit a similar error.

Firebase.crashlytics.setCrashlyticsCollectionEnabled(false)

And the rule isn't running, missing an evaluate here.

    override fun apply(base: Statement, description: Description): Statement {
        return object : Statement() {
            override fun evaluate() {
                RuleChain.outerRule(testName)
                    .around(composeTestRule)
                    .apply(base, description)
                    .evaluate()
            }
        }
    }
yschimke commented 1 year ago

I don't think you can put the test functionality in androidMain. I think it needs to be in a test only dependency.

yschimke commented 1 year ago

You'll also need

debugImplementation(libs.compose.ui.manifest)
yschimke commented 1 year ago

image

oas004 commented 1 year ago

Oh wow! Thanks man! That explains also why the printing statements never worked. I was so sure that that was AS messing with meπŸ˜…

Does that mean I should make a new module that only share test related code between wear and androidApp that both have testImplementation on?

yschimke commented 1 year ago

Yep, a new module

yschimke commented 1 year ago

Also we should try to run these without koinapp. Not sure why I used that everywhere.

oas004 commented 1 year ago

I think I managed to remove koinTest stuff from it now. But I had to comment out the firebase stuff. Not quite sure why πŸ€”

In terms of API, does it make sense to separate it out to a wear test rule? Just thinking since they are using a lot of dependencies from Horologist from the baseClass. How would the ideal way to separate this be? Maybe a testRule that extends the one in the android test module?

yschimke commented 1 year ago

Let's use roborazzi when ready