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

GMD recordMode not working as expected #244

Closed
1 of 5 tasks
matthew-shin-hs opened this issue Feb 10, 2025 · 29 comments · Fixed by #248
Closed
1 of 5 tasks

GMD recordMode not working as expected #244

matthew-shin-hs opened this issue Feb 10, 2025 · 29 comments · Fixed by #248
Labels
Bug Any behaviour the deviates from the documentation, expected outcome or API contract.

Comments

@matthew-shin-hs
Copy link

matthew-shin-hs commented Feb 10, 2025

Setting up GMD, followed the instructions in both the blog and the recipe section but can't seem to get the second way of recording baseline to work. By putting recordMode true in the build.gradle file instead of through the test configure method.

This issue relates to:

  • The Kotlin library
  • The Gradle plugin
  • The IntelliJ Platform plugin
  • The sample code
  • The documentation

To Reproduce
Steps to reproduce the behavior:
Try setting the recordMode to true and record baseline

Expected behavior
New baseline are recorded and do not get failures in build with the dev.testify.core.exception.ScreenshotIsDifferentException:

Desktop (please complete the following information):

  • OS: macOS
  • Version 15.3

Target Android Device (please complete the following information):

 testOptions {
        managedDevices {
            devices {
                  screenshotTest(ManagedVirtualDevice) {
                      device = "Pixel 4"
                      apiLevel = 34
                      systemImageSource = "aosp"
                 }
            }
        }
    }

Additional context

    testify {
        applicationPackageId "..."
        testPackageId "..."
        useTestStorage true
        recordMode true
    }
@matthew-shin-hs matthew-shin-hs added the Bug Any behaviour the deviates from the documentation, expected outcome or API contract. label Feb 10, 2025
@DanielJette
Copy link
Contributor

It seems we're missing tests for this scenario so it might have regressed. Thank you for bringing this to our attention. We'll take a look for the next release.

@matthew-shin-hs
Copy link
Author

Thanks @DanielJette. Do you have a timeline for when that next release may be? Our CI/CD pipeline has really flaky tests when we use the emulator that it comes installed with so I was hoping to try and migrate to GMD for our test suite to see if there would be any improvement. Currently had to disable all our tests... 🙁

@DanielJette
Copy link
Contributor

We don't have a schedule for the next release, but I can take a look at this issue tonight to see if it's a quick fix or not. I'll update this ticket tomorrow morning with an estimate on when you can expect a fix.

@DanielJette
Copy link
Contributor

@matthew-shin-hs I was looking at this today and I'm not sure if I'm able to correctly reproduce your issue. When you say it's not working, can you elaborate? Does it run the test correctly?
Also, what gradle command are you invoking?

@matthew-shin-hs
Copy link
Author

@DanielJette I'm running ./gradlew app:myDeviceDebugAndroidTest but I named my device screenshotTest so ./gradlew app:screenshotTestDebugAndroidTest. I am also testing in only one of my modules so ./gradlew :someModule:myDeviceDebugAndroidTest.

It runs the test instead of recording baseline like I expect it to when I set in the build.gradle to include

testify {
    recordMode = true
}

I am also expecting the test itself to pass but I see some fail, but I am guessing it is because I haven't correctly set up my device with my existing screenshot baselines. But I would only know that's the case if I were able to record a new baseline and see the diff on it which I cannot because I can't get it to record a new baseline at all. Either through the recordMode = true in the build.gradle or by manually adding in the tests:

rule
        .configure { 
            isRecordMode = true
        }
        .assertSame()

I hope this clarifies!

@DanielJette
Copy link
Contributor

Thanks for the clarification @matthew-shin-hs

I've been able to narrow down the issue to an update made to the GMD runtime. Previously, GMD tests would execute the same instrumented code path as an Android Test. This no longer appears to be the case. I'm not sure when this change happened, but the consequence of this is that the testify {} closure values are not being correctly passed to the library when invoking GMD tests.

I'm going to investigate alternatives and hope to have a solution up shortly.

DanielJette added a commit that referenced this issue Feb 17, 2025
Update ManifestHelper to use alternate version of getApplicationInfo
@DanielJette
Copy link
Contributor

Fix is here #248
Will try to get this published tomorrow.

DanielJette added a commit that referenced this issue Feb 17, 2025
Update ManifestHelper to use alternate version of getApplicationInfo
@matthew-shin-hs
Copy link
Author

@DanielJette perfect thank you! I will be on the lookout for the new version with the fix!

DanielJette added a commit that referenced this issue Feb 18, 2025
Update ManifestHelper to use alternate version of getApplicationInfo
DanielJette added a commit that referenced this issue Feb 19, 2025
* 246: Upgrade to AGP 8.8 and Gradle 8.10.2

* 244: Fix #244 GMD recordMode not working as expected

Update ManifestHelper to use alternate version of getApplicationInfo

* Fix "Unresolved reference: serviceOf"

* Fix "Inconsistent JVM-target compatibility detected for tasks 'compileJava' (21) and 'compileKotlin' (17)."
@DanielJette
Copy link
Contributor

@matthew-shin-hs 3.2.1 has been published. Please let me know if this has addressed your issue adequately.

@matthew-shin-hs
Copy link
Author

matthew-shin-hs commented Feb 19, 2025

@DanielJette Hey thanks for the update. I tried it out real quick but I get this error:

 java.lang.IncompatibleClassChangeError: The method 'android.net.Uri androidx.test.services.storage.TestStorage.getOutputFileUri(java.lang.String)' was expected to be of type static but instead was found to be of type virtual (declaration of 'dev.testify.output.TestStorageDestination' appears in /data

I have both

testify {
    useTestStorage = true
}

and

android {
    defaultConfig {
        testInstrumentationRunnerArguments useTestStorageService: "true"
    }
}

and the dependency for the test service that is outlined in the doc. I've also tried in both recordMode true and false but the same results come up

So I'm not sure what else I'm missing. I'll keep digging but wondering if this is also potentially a bug.

@DanielJette
Copy link
Contributor

Oh, sorry about that @matthew-shin-hs
That's caused by #243
I'll prioritize the fix for that and try to get that into a new release ASAP.

A workaround would be to drop the version of your androidx.test.services:storage dependency to 1.4.0 as Google moved a method from static to a class member.

@matthew-shin-hs
Copy link
Author

@DanielJette Oh gotcha thanks for the heads up. Thank you! Always appreciate the quick response. I'll try the workaround and let you know

@matthew-shin-hs
Copy link
Author

matthew-shin-hs commented Feb 19, 2025

@DanielJette Oh I see in the doc androidx.test.services:test-services not androidx.test.services:storage. I've tried with 1.4.0 on the test-services and it was giving me the same error. Trying invalidate cache restart as a sanity check..

@matthew-shin-hs
Copy link
Author

@DanielJette hmm I was able to get it to run with 1.4.0 but I had to change a bunch of other dependencies on androidx.test libraries to make it work with 1.4.0. However, when I do run it, it just passes the tests and does not record new baselines. I've also tried to remove the references all together so that it has no references and it still does not record it and just "passes". If I remove the recordMode true, it will correctly fail the tests showing that the baseline is not found.

@DanielJette
Copy link
Contributor

@matthew-shin-hs I'm going to try to update testify with the proper 1.5.0 dependencies tonight so hopefully I can get you a fixed release tomorrow.

As for the baseline, I debugged this a little while I was working on this fix and I noticed that the GMD device has a different instrumentation runner than typical android tests and this means that the console output isn't shown on the host computer. It should be recording the baseline images, it's likely that you're just not seeing any updates.

You can confirm this by checking the logcat output. For example:

When I run ./gradlew GmdSample:testerDebugAndroidTest, I see the following logcat:

~/dev/android-testify: adb logcat | grep ScreenshotUtility
02-16 10:49:38.585  2379  2399 D ScreenshotUtility: Writing screenshot to {/data/user/0/dev.testify.sample.gmd/app_images/screenshots/30-1080x1920@420dp-en_US/ClientListActivityScreenshotTest_testMissingBaseline.png}
02-16 10:49:38.639  2379  2399 E AssetLoader: 	at dev.testify.ScreenshotUtilityKt.loadBitmapFromAsset(ScreenshotUtility.kt:101)
02-16 10:49:38.639  2379  2399 E AssetLoader: 	at dev.testify.ScreenshotUtilityKt.loadBaselineBitmapForComparison(ScreenshotUtility.kt:120)
02-16 10:49:39.725  2379  2399 D ScreenshotUtility: Writing screenshot to {/data/user/0/dev.testify.sample.gmd/app_images/screenshots/30-1080x1920@420dp-en_US/ClientListActivityScreenshotTest_testRecorded.png}
02-16 10:49:40.955  2379  2399 D ScreenshotUtility: Writing screenshot to {/data/user/0/dev.testify.sample.gmd/app_images/screenshots/30-1080x1920@420dp-en_US/ClientListActivityScreenshotTest_default.png}

@DanielJette
Copy link
Contributor

When in GMD mode, Testify isn't able to automatically update your assets under source control. After the tests are run, you should find the baseline images have been copied to an build/outputs/managed_device directory.

For example:

Wed Feb 19 19:52:15
~/dev/android-testify: filegrep managed_device                   
./Samples/Gmd/build/outputs/managed_device_code_coverage
./Samples/Gmd/build/outputs/managed_device_code_coverage/debug
./Samples/Gmd/build/outputs/managed_device_code_coverage/debug/screenshotTest
./Samples/Gmd/build/outputs/managed_device_code_coverage/debug/tester
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester/images
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester/images/screenshots
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester/images/screenshots/30-1080x1920@420dp-en_US
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester/images/screenshots/30-1080x1920@420dp-en_US/ClientListActivityScreenshotTest_testRecorded.png
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester/images/screenshots/30-1080x1920@420dp-en_US/ClientListActivityScreenshotTest_default.png
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester/images/screenshots/30-1080x1920@420dp-en_US/ClientListActivityScreenshotTest_testMissingBaseline.png
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester/testify
./Samples/Gmd/build/outputs/managed_device_android_test_additional_output/debug/tester/testify/report.yml

@DanielJette
Copy link
Contributor

We'd hoped to add native support for copying files via the screenshotPull command, but as of yet, no one has tried to implement it

#174

@matthew-shin-hs
Copy link
Author

@DanielJette ah yes thanks so much. I remember reading that with the managed outputs but had forgotten about it since I last mentioned the bug. I check that output and it's there! So that's great. With your fix for 1.5.0 support, even better.

Just curious, with GMD, because it launches a new emulator each time it runs, it shouldn't depend on any VM pre installed on it, is that correct?

@DanielJette
Copy link
Contributor

Just curious, with GMD, because it launches a new emulator each time it runs, it shouldn't depend on any VM pre installed on it, is that correct?

Yes, my understanding is that it's fully self-contained. I have a new Mac Mini and only installed Android Studio and the single default emulator that came with it and yet it provisioned and launched the GMD device without any prompting from me.

@matthew-shin-hs
Copy link
Author

@DanielJette and one other question I couldn't find references to in the docs, I know we should disable animations through the emulators to minimize differences on each test run, is that the same case for GMD? Assuming that's another benefit of using GMD? So that locally or through CI/CD you don't have to manually set these emulator settings and instead can be done through gradle?

@DanielJette
Copy link
Contributor

@DanielJette and one other question I couldn't find references to in the docs, I know we should disable animations through the emulators to minimize differences on each test run, is that the same case for GMD? Assuming that's another benefit of using GMD? So that locally or through CI/CD you don't have to manually set these emulator settings and instead can be done through gradle?

I haven't actually noticed if the animations are enabled or not, but I would agree that it seems logically to expect that they would be.

To be safe, you can also set animationsDisabled in your build.gradle file:

https://developer.android.com/reference/tools/gradle-api/7.4/com/android/build/api/dsl/TestOptions#animationsDisabled()

testOptions {
  animationsDisabled true
}

@DanielJette
Copy link
Contributor

DanielJette commented Feb 20, 2025

@matthew-shin-hs I just published a 3.2.2-alpha01 for you to test that has the TestStorage fix. If that works for you, I'll prepare the official release.

It'll probably take an hour or so for the artifacts to appear on Maven https://repo1.maven.org/maven2/dev/testify/testify/

@matthew-shin-hs
Copy link
Author

matthew-shin-hs commented Feb 20, 2025

@DanielJette Yes! It looks like that release works with the latest version of test-services. I have noticed from recording though that I get the following error from the log from time to time. It results in some tests not recording, and re-running it usually solves the problem. I'm not sure exactly what's causing it. I'm not sure if this is a separate bug or an issue or I'm missing something in terms of settings.

Image

UPDATE: I've tried to run this a couple more times to get a consistent failure to reproduce it, but I can't quite figure out exactly what's causing it. Sometimes it works, sometimes it doesn't? It also seems like that sometimes even when I get that error, things still record the way I expect it to. I didn't spend too much time on it in case it is a known bug, but if you are not aware of it I can look more into the details to get a consistent reproduction tomorrow

UPDATE 2: I've also tried using the gradlew command with --no-parallel but seems to be same

@matthew-shin-hs
Copy link
Author

matthew-shin-hs commented Feb 20, 2025

@DanielJette

We'd hoped to add native support for copying files via the screenshotPull command, but as of yet, no one has tried to implement it

#174

From the doc, it says:

As we cannot use screenshotPull task, when test execution is completed, the Test Storage service will copy recorded files from the module's build/outputs/managed_device_android_test_additional_output/myDevice into your androidTest/assets/screenshots/ directory.

Doesn't this imply that it will automatically copy the recorded files to the androidTest/assets/screenshots/ directory? I think you have to manually do this right now?

@DanielJette
Copy link
Contributor

@matthew-shin-hs About the error, I double-checked the code and that is a handled exception. Testify is logging it as an error, but it shouldn't be resulting in any test failures. That FileNotFound is referencing the assets baseline image and it would make sense for this to fail if you're recording a new screenshot. To reproduce, delete the baseline images in assets/screenshots and re-run record mode. You should expect to see the error in the logs, but the test should not fail.

Arguably, we could have added a file.exists() check to this code and skip trying to load it...

@DanielJette
Copy link
Contributor

Doesn't this imply that it will automatically copy the recorded files to the androidTest/assets/screenshots/ directory? I think you have to manually do this right now?

@matthew-shin-hs That's definitely what this says, but the code definitely does not do this. The Testify library (where GMD support is implemented) can not access the host computer. It runs on the managed device/emulator only and so it's impossible for it to automatically copy files to the assets directory under version control. This is why we use TestStorage as it's a bridge between the emulator and the host computer's file system. So, more accurately, Testify copies from the data/data directory on the emulator into the build/outputs/managed_device on the local computer.

That's definitely incorrect information in the documentation and it will have to be updated.

I've logged a bug against the documentation here: #250

@matthew-shin-hs
Copy link
Author

@DanielJette perfect, I think it looks good, thanks for addressing the bug so quickly! Last thing, I'm assuming this gradle setup has to be done for each module in a project that's multi-modular instead of in like a root level build.gradle?

@djette-st
Copy link
Contributor

@matthew-shin-hs Oh, great question. I haven't actually tried this, but I would presume those settings are per-module.

@matthew-shin-hs
Copy link
Author

matthew-shin-hs commented Feb 20, 2025

@DanielJette @djette-st Perfect, thanks so much for your help. I'll keep a look out for the official 3.2.2 release! If you could let me know the rough estimate timeline on the estimate that would also be super helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Any behaviour the deviates from the documentation, expected outcome or API contract.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants