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

Sanity tests robot pattern and reliability refactor #4420

Merged
merged 19 commits into from
Nov 8, 2021

Conversation

ouchadam
Copy link
Contributor

@ouchadam ouchadam commented Nov 5, 2021

  • Ports the sanity test to the robot pattern (extracting the UI navigation and manipulation to dedicated classes which return themselves back to the original state), this enables fine grain reusable navigation
  • Swaps out timeouts for view waits where possible
  • Adds missing view waits to make the tests less flaky (they now consistently pass for me with an emulator)
  • Disables the sign out flow as the tests currently hit a bug
GIF SIGN OUT BUG
after-sanity Screenshot_20211104_163044

@@ -70,6 +73,8 @@ fun waitForView(viewMatcher: Matcher<View>, timeout: Long = 10_000, waitForDispl
val endTime = startTime + timeout
val visibleMatcher = isDisplayed()

uiController.loopMainThreadForAtLeast(100)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really wanted to avoid this but this single change made the biggest difference for me when it came to reliability.

Dialogs and bottomsheets would be visible but not interactable, back actions or button presses would be ignored half the time~.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this nice rework.
A few feedbacks

// Disable until the "you don't have a session for id %d" sign out bug is fixed
// elementRobot.signout()
//// Login again on the same account
// elementRobot.login(userId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not test regular login then... This is a bit annoying

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed! I'd like to look into the bug and reenable this ASAP

import im.vector.app.features.home.room.detail.RoomDetailActivity

class CreateNewRoomRobot(
var createdRoom: Boolean = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the goal of this boolean? (I do not know the Robot pattern)

Copy link
Contributor Author

@ouchadam ouchadam Nov 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's to avoid a pressBack if we create a new room as navigating to the new room finishes the current room creation activity (I hope that makes sense? 😅 )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see. But this test should always create a room, so this is still strange to me.

BaristaClickInteractions.clickOn(R.string.create_new_room)
val createNewRoomRobot = CreateNewRoomRobot()
block(createNewRoomRobot)
if (!createNewRoomRobot.createdRoom) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test should fail in this case, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this the next steps will fail as every robot assumes the start and end position are the same

in the case of creating a new room the room creation screen is automatically finished so there's no need to press back, if we did then we'd exit the app as the RoomDetails

// move to room list tab and press #+
elementRobot.newRoom {
    // press create new room launches CreateRoomActivity
    createNewRoom {
        //  press create, navigates to the RoomDetailsActivity, CreateRoomActivity automatically closes
        createRoom {
            // goes through all the room detail checks
            crawl()             
            // on createRoom finish pressBack to take us back to root activity
        }
        // avoid pressing back because we created a room
    }
    // avoid pressing back because we created a room
}

@ouchadam
Copy link
Contributor Author

ouchadam commented Nov 5, 2021

some observations during my testing

  • Sometimes I see a concurrent exception during the login flow, seems to be coming from the testing library
2021-11-05 14:45:28.209 12441-12441/im.vector.app.debug E/MonitoringInstr: Exception encountered by: im.vector.app.features.login.LoginActivity@c5555e9. Dumping thread state to outputs and pining for the fjords.
    java.util.ConcurrentModificationException
        at java.util.ArrayList$Itr.next(ArrayList.java:860)
        at androidx.test.internal.runner.lifecycle.ActivityLifecycleMonitorImpl.signalLifecycleChange(ActivityLifecycleMonitorImpl.java:159)
        at androidx.test.runner.MonitoringInstrumentation.callActivityOnStop(MonitoringInstrumentation.java:754)
        at android.app.Activity.performStop(Activity.java:8018)
        at android.app.ActivityThread.callActivityOnStop(ActivityThread.java:4616)
        at android.app.ActivityThread.performStopActivityInner(ActivityThread.java:4594)
        at android.app.ActivityThread.handleStopActivity(ActivityThread.java:4669)
        at android.app.servertransaction.TransactionExecutor.performLifecycleSequence(TransactionExecutor.java:233)
        at android.app.servertransaction.TransactionExecutor.cycleToPath(TransactionExecutor.java:201)
        at android.app.servertransaction.TransactionExecutor.executeLifecycleState(TransactionExecutor.java:173)
        at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:97)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2016)
        at android.os.Handler.dispatchMessage(Handler.java:107)
        at android.os.Looper.loop(Looper.java:214)
  • On my emulator the EditTextPreference causes an OS tombstone, this happens in the display name dialog but also when viewing the System android build version 🤔

@github-actions
Copy link

github-actions bot commented Nov 5, 2021

Unit Test Results

  62 files  ±0    62 suites  ±0   1m 9s ⏱️ +4s
118 tests ±0  118 ✔️ ±0  0 💤 ±0  0 ±0 
350 runs  ±0  350 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 78675d4. ± Comparison against base commit be932a8.

♻️ This comment has been updated with latest results.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update.
I'll trust your judgment for the boolean var in the room creation robot.

@bmarty bmarty merged commit cea2206 into develop Nov 8, 2021
@bmarty bmarty deleted the feature/adm/sanity-check-robot branch November 8, 2021 13:54
@bmarty
Copy link
Member

bmarty commented Nov 8, 2021

ktlint issues fixed on develop in 2b58c0e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants