nowinandroid icon indicating copy to clipboard operation
nowinandroid copied to clipboard

[Bug]: DemoDebug Local test failed

Open Jaehwa-Noh opened this issue 11 months ago • 20 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Is there a StackOverflow question about this issue?

  • [X] I have searched StackOverflow

What happened?

Run local tests on DemoDebug variant: ./gradlew testDemoDebug has failed test on the barely fork the repository.

I entered the command in the Execute Gradle Task on android studio. image

But the test does not success, see below. image

Any thing I missed? or is this already known issue?

Relevant logcat output

Output is so long, I'd add partially.

java.lang.IllegalArgumentException: resource native/windows/x86_64/robolectric-nativeruntime.dll not found.

java.io.IOException: Unable to rename C:\Users\Jaehwa\AppData\Local\Temp\junit4146744981157378858\user_preferences_test.pb.tmp.This likely means that there are multiple instances of DataStore for this file. Ensure that you are only creating a single instance of datastore for this file.

14 tests completed, 4 failed
14 tests completed, 4 failed


        Caused by: java.io.IOException at NiaPreferencesDataSourceTest.kt:92

        Caused by: java.io.IOException at NiaPreferencesDataSourceTest.kt:74

        Caused by: java.io.IOException at NiaPreferencesDataSourceTest.kt:53

        Caused by: java.io.IOException at NiaPreferencesDataSourceTest.kt:92

        Caused by: java.io.IOException at NiaPreferencesDataSourceTest.kt:74

        Caused by: java.io.IOException at NiaPreferencesDataSourceTest.kt:53

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

Jaehwa-Noh avatar Mar 04 '24 04:03 Jaehwa-Noh

Are You a Windows OS User? Now in Android screenshot test library uses Roborazzi. This library uses Robolectric NATIVE GraphicsMode internally, but this does not support Windows OS and seems to cause the following error

java.lang.IllegalArgumentException: resource native/windows/x86_64/robolectric-nativeruntime.dll not found.

So, the local screenshot tests would fail on Windows OS. ref: https://github.com/robolectric/robolectric/issues/8312

I am not sure of the exact cause of the problem with the DataStore, but I have encountered the same error, too.

sanao1006 avatar Mar 04 '24 05:03 sanao1006

Yes, I'm use a Windows OS. Thank you, I didn't know Roborazzi doesn't support Windows OS. You say, encountered same error with DataStore, it gives me relief.

Jaehwa-Noh avatar Mar 04 '24 05:03 Jaehwa-Noh

Roborazzi doesn't support Windows OS.

I think it would be correct to express that Roborazzi does not support Windows OS, but rather Robolectric NATIVE GraphicsMode does not support Windows OS!

sanao1006 avatar Mar 04 '24 05:03 sanao1006

Yes, I agree with that.

So, the local screenshot tests would fail on Windows OS. ref: https://github.com/robolectric/robolectric/issues/8312

I checked that word from yours link.

Jaehwa-Noh avatar Mar 04 '24 05:03 Jaehwa-Noh

The issue with Roborazzi and DataStore are two different issue. For the DataStore issue, you can read more on this issue (especially the last comment with links):

  • https://github.com/android/nowinandroid/issues/98#issuecomment-1320886904

SimonMarquis avatar Mar 04 '24 09:03 SimonMarquis

From your Issues and relative links, I finally landed this page. Might be the datastore problem will be solved when the next DataStore version release. Thank you.

Jaehwa-Noh avatar Mar 04 '24 11:03 Jaehwa-Noh

Robolectric 4.11 adds support for the native runtime (native SQLite and graphics modes) on Windows. https://github.com/robolectric/robolectric/releases/tag/robolectric-4.12

SimonMarquis avatar Mar 30 '24 06:03 SimonMarquis

@SimonMarquis I recently tried to use Roborazzi in my app but it get stucked 😅 when I ran it on my machine. Maybe Roborazzi needs to update too or I'm the only one experiencing this.

- Windows 10 - Iguana

JackEblan avatar Mar 30 '24 09:03 JackEblan

Unfortunately there still seems to be issues with windows runners... Like this one:

java.nio.file.FileSystemException: C:\Users\RUNNER~1\AppData\Local\Temp\robolectric-nativeruntime12669726669700075741\fonts\CarroisGothicSC-Regular.ttf: The process cannot access the file because it is being used by another process

https://github.com/SimonMarquis/nowinandroid/actions/runs/8488861060/job/23258373077#step:13:1

SimonMarquis avatar Mar 30 '24 18:03 SimonMarquis

I was unable to reproduce this issue in roborazzi repository. Therefore, I attempted to use the nowinandroid repository and tried fixing it with ActivityScenario.close, but it seems to be ineffective. I also reviewed the Robolectric repository and noticed that @hoisie addressed a similar issue last week. https://github.com/robolectric/robolectric/commit/3b90118de04963282bc2389544ccfdc194dd1945#diff-31315cbee32dff759fd6d17e6d8939e67a292dc9e6c05591e4b95462a5ef1c43R44

@hoisie It might seem a bit abrupt, but are you aware of this?

takahirom avatar Mar 31 '24 08:03 takahirom

Robolectric 4.11 adds support for the native runtime (native SQLite and graphics modes) on Windows. https://github.com/robolectric/robolectric/releases/tag/robolectric-4.12

Just to be clear, this was a typo, and should read 4.12 release.

Windows seems to be a lot more protective of files that have been opened in native code. For instance, unlike in Mac and Linux, it's not possible to delete the temp native runtime DLL file in the same process where it was opened. We are seeing something similar with font files too. This is why we have extra cleanup logic for Windows.

hoisie avatar Mar 31 '24 15:03 hoisie

I am not sure that the FileSystemException errors you are seeing are causing test failures:

ERROR: Failed to destroy temp directory

java.nio.file.FileSystemException: C:\Users\RUNNER~1\AppData\Local\Temp\robolectric-nativeruntime12669726669700075741\fonts\CarroisGothicSC-Regular.ttf: The process cannot access the file because it is being used by another process

This deletion logic occurs in a shutdown hook, so it would run after all tests are complete. I think we should completely remove these error messages on Windows, because they will cause a lot of noise and confusion.

I think that the tests are failing due to other reasons.

hoisie avatar Mar 31 '24 16:03 hoisie

You are probably right. I can't test it further since I don't have my Windows machine with me, but the errors seem to come from other IO Exceptions.

com.google.samples.apps.nowinandroid.ui.NiaAppScreenSizesScreenshotTests > expandedWidth_compactHeight_showsNavigationRail FAILED
    java.io.IOException at SingleProcessDataStore.kt:433
        Caused by: java.io.IOException at NiaAppScreenSizesScreenshotTests.kt:126

SimonMarquis avatar Mar 31 '24 17:03 SimonMarquis

Renders with Windows runners are different for the NavigationBar / NavigationRail tests:

https://github.com/SimonMarquis/nowinandroid/pull/24/commits/6e4e7b5d56058a809561de9141f68624f37b3a0d

Screenshots
compactWidth_compactHeight_showsNavigationBar_compare
compactWidth_expandedHeight_showsNavigationBar_compare
compactWidth_mediumHeight_showsNavigationBar_compare
expandedWidth_compactHeight_showsNavigationRail_compare

SimonMarquis avatar Apr 01 '24 11:04 SimonMarquis

https://github.com/robolectric/robolectric/releases/tag/robolectric-4.12.1 was just released which should reduce the amount of log noise in Windows (though it won't fix the underlying cause of this issue).

RE comparing screenshots across platforms (i.e. Linux / Mac / Linux): There are no guarantees that rendering will be consistent across different platforms. The underlying native libraries (e.g. skia, minikin, etc..) have platform-specific implementation details.

hoisie avatar Apr 02 '24 19:04 hoisie

Could this be an issue related to Looper/Dispatcher that would not behave exactly the same? I feel like the screenshots are not rendering the expected "state".

There are some state setup in @Before that does not seem to be applied on Windows (hence the screenshot differences)

https://github.com/android/nowinandroid/blob/88c3eb0b90ace5499962d9495464fa2388ec9e9a/app/src/testDemo/kotlin/com/google/samples/apps/nowinandroid/ui/NiaAppScreenSizesScreenshotTests.kt#L125-L132

SimonMarquis avatar Apr 02 '24 22:04 SimonMarquis

@Jaehwa-Noh a fix has been implemented in #1542 for your initial bug report, can you check if the fix works for you as well?

SimonMarquis avatar Jul 11 '24 21:07 SimonMarquis

This might also fix the secondary issue, let us know.

SimonMarquis avatar Jul 11 '24 21:07 SimonMarquis

Okay I'll test it as soon as possible.

Jaehwa-Noh avatar Jul 12 '24 04:07 Jaehwa-Noh

@SimonMarquis https://github.com/android/nowinandroid/pull/1542#pullrequestreview-2174456503 It works.

Jaehwa-Noh avatar Jul 12 '24 11:07 Jaehwa-Noh