Issue 121/run fix tests #371

Nikitae57 · 2022-07-06T14:25:57Z

Decided to split this task into two: fix broken tests and configure CI. This PR contains test fixes

RuslanMingaliev · 2022-07-13T11:40:56Z

kaspresso/src/main/kotlin/com/kaspersky/kaspresso/device/screenshots/ScreenshotsImpl.kt

@@ -28,6 +28,8 @@ class ScreenshotsImpl(

    override fun takeAndApply(tag: String, block: File.() -> Unit): Unit = doTakeAndApply(tag, block)

+    override fun getScreenshotDir(): File = resourceFilesProvider.provideScreenshotFile("").parentFile as File


Ugly hack. Add a provideScreenshotDir function to the resourceFilesProvider

RuslanMingaliev · 2022-07-13T11:43:37Z

...androidTest/kotlin/com/kaspresky/kaspresso/composesupport/sample/screen/ComposeMainScreen.kt

@@ -1,4 +1,4 @@
-package com.kaspersky.kaspresso.composesupport.sample.screen
+package com.kaspresky.kaspresso.composesupport.sample.screen


RuslanMingaliev · 2022-07-13T11:47:23Z

.../src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceLocationSampleTest.kt

-                    MUNICH_LOCATION_LAT, location.latitude,
-                    DELTA
-                )
+                flakySafely(timeoutMs = 10_000, intervalMs = 500) {


Checking for non-null is not actual now

RuslanMingaliev · 2022-07-13T11:48:56Z

...le/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceLogcatSampleTest.kt

@@ -16,6 +18,8 @@ class DeviceLogcatSampleTest : TestCase() {

    @Test
    fun logcatTest() {
+        Assume.assumeTrue(Build.VERSION.SDK_INT != Build.VERSION_CODES.O) // "logcat -c" fails on android 8 for some reason


We should fix it or describe this limitation in the docs (and throw an exception)

RuslanMingaliev · 2022-07-13T11:49:10Z

...le/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceLogcatSampleTest.kt

@@ -16,6 +18,8 @@ class DeviceLogcatSampleTest : TestCase() {

    @Test
    fun logcatTest() {
+        Assume.assumeTrue(Build.VERSION.SDK_INT != Build.VERSION_CODES.O) // "logcat -c" fails on android 8 for some reason


Suggested change

Assume.assumeTrue(Build.VERSION.SDK_INT != Build.VERSION_CODES.O) // "logcat -c" fails on android 8 for some reason

Assume.assumeTrue(Build.VERSION.SDK_INT != Build.VERSION_CODES.O) // "logcat -c" fails on Android 8 for some reason

RuslanMingaliev · 2022-07-13T11:51:21Z

...rc/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceScreenshotSampleTest.kt

-                assertFalse(screenshotExists())
-                device.screenshots.take(SCREENSHOT_TAG)
-                assertTrue(screenshotExists())
+                assertTrue(screenshotsDir.list()?.isEmpty() ?: true)


Intention of the assertion has been changed

How? We check that screenshot folder is empty, take screenshot and check that it exists as we did before

RuslanMingaliev · 2022-07-13T11:52:35Z

...src/androidTest/kotlin/com/kaspersky/kaspressample/docloc_tests/ChangeSysLanguageTestCase.kt

-            Thread.sleep(3000)
-        }
+    fun showAndroidSettings() {
+        Assume.assumeTrue(Build.VERSION.SDK_INT <= Build.VERSION_CODES.O) // Can't change system locale on newer versions


We should fix it or describe this limitation in the docs

Maybe throw an exception too?

matzuk · 2022-07-20T08:09:23Z

gradle/libs.versions.toml

@@ -6,6 +6,7 @@ allure = "2.4.0"
 compose = "1.0.5"
 activityCompose = "1.4.0"
 androidXTest = "1.4.0"
+testOrchestrator = "1.4.1"


Do we need testOrchestrator?
I've (and not only me) observed a lot of cases when Orchestrator "doesn't see" tests while you try to run some bunch of them. Temporary removing of Orchestrator from the dependencies and usage of base AndroidTestRun resolved the issue for a lot of cases. Let's try to remove Orchestrator because anyway we use Marathon or something similar for more smart tests execution.

@Nikitae57 Were there any problems with the app state being shared between tests inside the test run?

@matzuk I've encountered this behavior too, but it got resolved by upgrading orchestrator from 1.4.0 to 1.4.1.
@eakurnikov Yes, without orchestrator some tests worked fine when I launched them in scope of their module (gradlew moduleName:connectedDebugAndoirdTest) but failed when I run them in all modules (gradlew connectedAndroidDebugTest)

1.4.0 doesn't work with 31 api at all. Even after the update orchestrator remains flaky especially when it comes to local test run. But I think it is fine to have it here for now. We can add other runner in future.

matzuk · 2022-07-20T08:10:39Z

...so-sample/src/androidTest/kotlin/com/kaspersky/kaspressample/compose_tests/WebComposeTest.kt

@@ -39,22 +39,22 @@ class WebComposeTest : TestCase() {
                }
            }

-            step("Find \"Sign in\" button and \"Protect your data\" title") {
+            step("Find \"Sign in\" button and title") {


Maybe it makes sense to create our own simple WebPage?

Yeah, It make sense. I'll work on that

matzuk · 2022-07-20T08:12:18Z

...e/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceExploitSampleTest.kt

@@ -54,9 +55,10 @@ class DeviceExploitSampleTest : TestCase() {
                    Configuration.ORIENTATION_LANDSCAPE ==
                            activityTestRule.activity.resources.configuration.orientation
                }
+                Screen.idle() // kaspresso misses the button in case we won't wait


Interesting. Kaspresso should handle all possible stuff in related Interceptors. Are you sure that everything is fine here?

@Nikitae57 Can you share more context on this? I am not able to reproduce this issue, for me test passes. And I don't see how is it possible actually to miss a button as flaky safety should solve this problem.

I also changed the button id to wrong one to make sure the flaky safety works and it works:

Kaspresso didn't handle this in a "flaky safety" way because it kinda worked right. I encountered a scenario when kaspresso didn't wait for orientation change, tried to press button but missed as orientation change performed at this moment. I assume that changing orientation command sends signal to OS and returns control immediately before actual orientation change happens

I still don't understand how is it possible that flaky safety can't solve this. Any PerformException that was thrown in case of click failure would be handled anyway. Maybe there was some other exception that was not handled by flaky safety intentionally? For me this test passed successfully any time I launched it. I don't think that we should add Screen.idle() here. If you will be able to reproduce the problem and will provide more details, we could try to solve this in a proper way.

matzuk · 2022-07-20T08:13:14Z

...le/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceLogcatSampleTest.kt

-                val isContainsBreaked = device.logcat.readLogcatRows { logcatRow ->
-                    inneContainsSize++
+                var innerContainsSize = 0
+                val doesContainBreaked = device.logcat.readLogcatRows { logcatRow ->


is instead of does?

Why? "Does" is a modal verb as an "is" and fits English rules

matzuk · 2022-07-20T08:14:14Z

wiki/05_Device.md

@@ -20,7 +20,7 @@ All examples are located in [device_tests](../samples/kaspresso-sample/src/andro
 11. `hackPermissions` provides the possibility of allowing any permission requests without default Android permission dialog. See the example [DeviceHackPermissionsSampleTest](../samples/kaspresso-sample/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceHackPermissionsSampleTest.kt).
 12. `exploit` allows to rotate device or press system buttons. See the example [DeviceExploitSampleTest](../samples/kaspresso-sample/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceExploitSampleTest.kt).
 13. `language` allows to switch language. See the example [DeviceLanguageSampleTest](../samples/kaspresso-sample/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceLanguageSampleTest.kt).
-14. `logcat` provides access to adb logcat. See the example [DeviceLogcatSampleTest](../samples/kaspresso-sample/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceLogcatSampleTest.kt). <br>
+14. `logcat` provides access to adb logcat. See the example [DeviceLogcatSampleTest](../samples/kaspresso-sample/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceLogcatSampleTest.kt). Please, notice that due to Android 8 bug this test won't run successfully on AVD with API 26 <br>


what about API > 26?

It runs ok on any API I've tried except for 26. The problem is exactly API 26

eakurnikov · 2022-07-20T08:56:54Z

kaspresso/src/main/kotlin/com/kaspersky/kaspresso/device/screenshots/Screenshots.kt

+    /**
+     * @return directory used to store screenshots
+     */
+    fun getScreenshotDir(): File


For what purpose is it added? First, I see the only usage in a sample test, second, it semantically doesn't belong to this interface and the implementation just proxies the call to resource provider.

If you really need to get the used directories in test runtime I would suggest to make the ResourcesDirsProvider accessible via the TestContext. It can be done by providing it to TestAssistantsProvider.

Ok, thanks, I'll fix that

eakurnikov

Pls see comments

…o previous commit (#121)

eakurnikov · 2022-07-28T14:39:18Z

kaspresso/src/main/kotlin/com/kaspersky/kaspresso/kaspresso/Kaspresso.kt

@@ -145,7 +145,8 @@ data class Kaspresso(
    internal val objectBehaviorInterceptors: List<ObjectBehaviorInterceptor>,
    internal val deviceBehaviorInterceptors: List<DeviceBehaviorInterceptor>,
    internal val stepWatcherInterceptors: List<StepWatcherInterceptor>,
-    internal val testRunWatcherInterceptors: List<TestRunWatcherInterceptor>
+    internal val testRunWatcherInterceptors: List<TestRunWatcherInterceptor>,
+    internal val resourcesRootDirsProvider: ResourcesRootDirsProvider


Maybe it is more useful to have an access to ResourceFilesProvider? If we need only root dirs we still can get them from the files paths.

eakurnikov · 2022-08-04T13:17:34Z

...androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceAccessibilitySampleTest.kt

@@ -35,7 +34,7 @@ class DeviceAccessibilitySampleTest : TestCase() {
    @get:Rule
    val activityRule = ActivityTestRule(DeviceSampleActivity::class.java, false, true)

-    @Test
+    // @Test


Did you forgot to remove it?

Nope, I forgot to leave a comment. I'm creating a CI for running UI tests for kaspresso PRs and unstable test is not ideal. I've tried increasing timeouts up to minute, but no luck.

Maybe @Ignore("FIXME") then? :)

Sure, it's better

eakurnikov · 2022-08-04T13:18:37Z

...androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceAccessibilitySampleTest.kt

@@ -50,12 +49,12 @@ class DeviceAccessibilitySampleTest : TestCase() {
                    device.targetContext.packageName,
                    SERVICE_CLASS_NAME
                )
-                assertTrueSafely { isAccessibilityServiceEnabled() }
+                assertTrueSafely(timeoutMs = 60_000L, intervalMs = 1_000L) { isAccessibilityServiceEnabled() }


Isn't 60 sec too much?

Yeah, looks like it's either working or not. I've experienced cases when 60secs wasn't enough

eakurnikov · 2022-08-04T13:18:59Z

...e/src/androidTest/kotlin/com/kaspersky/kaspressample/device_tests/DeviceExploitSampleTest.kt

@@ -55,13 +55,16 @@ class DeviceExploitSampleTest : TestCase() {
                    Configuration.ORIENTATION_LANDSCAPE ==
                            activityTestRule.activity.resources.configuration.orientation
                }
-                Screen.idle() // kaspresso misses the button in case we won't wait
+                // Screen.idle() // kaspresso misses the button in case we won't wait


Did you forgot to remove it?

Nikitae57 added 4 commits June 30, 2022 19:10

Fixed some UI tests (#121)

226151c

Fixed kaspresso-sample UI tests (#121)

dd9ef20

Forgot to add file (#121)

3ce85e1

Minor code cleanup (#121)

c375a28

Nikitae57 requested review from matzuk, AzamatCherchesov, eakurnikov and RuslanMingaliev as code owners July 6, 2022 14:25

Fixed lint (#121)

6819e69

RuslanMingaliev reviewed Jul 13, 2022

View reviewed changes

Addressed PR comments (#121)

3492846

matzuk reviewed Jul 20, 2022

View reviewed changes

eakurnikov reviewed Jul 20, 2022

View reviewed changes

eakurnikov requested changes Jul 20, 2022

View reviewed changes

Nikitae57 added 2 commits July 26, 2022 18:47

Implemented local web page for webview tests; forgot to add files int…

c651b1d

…o previous commit (#121)

Using resourcesRootDirsProvider directly to get screenshots dir (#121)

90333d8

eakurnikov reviewed Jul 28, 2022

View reviewed changes

Addressed PR comments (#121)

1d395bc

eakurnikov reviewed Aug 4, 2022

View reviewed changes

eakurnikov previously approved these changes Aug 4, 2022

View reviewed changes

Addressed PR comments (#121)

822684d

Nikitae57 dismissed eakurnikov’s stale review via 822684d August 4, 2022 13:28

eakurnikov approved these changes Aug 4, 2022

View reviewed changes

Nikitae57 merged commit 2bd0241 into master Aug 4, 2022

Nikitae57 deleted the issue-121/run-tests-on-prs branch August 4, 2022 13:54

		@@ -28,6 +28,8 @@ class ScreenshotsImpl(

		override fun takeAndApply(tag: String, block: File.() -> Unit): Unit = doTakeAndApply(tag, block)

		override fun getScreenshotDir(): File = resourceFilesProvider.provideScreenshotFile("").parentFile as File

		@@ -1,4 +1,4 @@
		package com.kaspersky.kaspresso.composesupport.sample.screen
		package com.kaspresky.kaspresso.composesupport.sample.screen

	Assume.assumeTrue(Build.VERSION.SDK_INT != Build.VERSION_CODES.O) // "logcat -c" fails on android 8 for some reason
	Assume.assumeTrue(Build.VERSION.SDK_INT != Build.VERSION_CODES.O) // "logcat -c" fails on Android 8 for some reason

Issue 121/run fix tests #371

Issue 121/run fix tests #371

Conversation

Nikitae57 commented Jul 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eakurnikov Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eakurnikov Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nikitae57 Jul 26, 2022 • edited Loading

Choose a reason for hiding this comment

eakurnikov Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eakurnikov left a comment

Choose a reason for hiding this comment

eakurnikov Jul 28, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eakurnikov Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eakurnikov Jul 20, 2022 •

edited

Loading

eakurnikov Jul 20, 2022 •

edited

Loading

Nikitae57 Jul 26, 2022 •

edited

Loading

eakurnikov Jul 20, 2022 •

edited

Loading

eakurnikov Jul 28, 2022 •

edited

Loading

eakurnikov Aug 4, 2022 •

edited

Loading