search

steroid-team / app

Helping you organise your day with ease and privacy.
1 stars 2 forks source link

fix/ItemViewActivityTest ? #110

Closed theo-damiani closed 3 years ago

theo-damiani commented 3 years ago

Hey!

I tried to fix the item activity test which was failing. I think the failure is caused by the alert dialog not closing. Sometimes the alert dialog is not closing "faster enough" and so the onView in the next line is not able to recover other layout ids. I ran multiples times locally and the test was failing some times but after the fix, I didn't get any failure, so maybe it is okay now.

I cleaned up also some tests.

lgraziano commented 3 years ago

I tried it but I still got an error from the notificationDeleteWorks :

androidx.test.espresso.NoMatchingRootException: Matcher 'is toast' did not match any of the following roots: [...]

theo-damiani commented 3 years ago

Thanks for the comments!

For me, tests are failing because sometimes the alert dialog doesn't close itself. So I just added .inRoot(isDialog()), before performing an action on it. Apparently, it helps, don't really know why.

I also added a custom Matchers atPositionCheckText(), so we can check if the text in a task is indeed correct. Before in the tests, we were checking only by calling onView(withId(R.id.layout_task_body)), and so if there were multiple tasks it was failing (because with multiples tasks, there are the same number of layout_task_body). So now we can check a task given its index in the recycler view. So we can work with multiple tasks at the same time.

YagoGG commented 3 years ago

Hey @theo-damiani, thanks for the explanation!

As we just discussed in the standup, here's the traceback I'm —still— getting (with your patch applied):

> Task :app:connectedDebugAndroidTest
Starting 9 tests on Pixel_3a_API_29_x86(AVD) - 10

com.github.steroidteam.todolist.ItemViewActivityTest > removeTaskButtonWorks[Pixel_3a_API_29_x86(AVD) - 10] FAILED 
    androidx.test.espresso.NoMatchingViewException: No views in hierarchy found matching: with id is <com.github.steroidteam.todolist:id/activity_itemview_itemlist>
YagoGG commented 3 years ago

And just for the record, I just double-checked and this is the same issue that appears in the CI when the build fails.

theo-damiani commented 3 years ago

I tried it but I still got an error from the notificationDeleteWorks :

androidx.test.espresso.NoMatchingRootException: Matcher 'is toast' did not match any of the following roots: [...]

Okay so after checking with Leandro the Toast Matcher indeed doesn't work with API 30 and Android Level 11. But no problem with API29 and Android 10.

I will test with the same as Yago Pixel_3a_API_29_x86(AVD) - 10, I have Pixel 2 API29

theo-damiani commented 3 years ago

Hey @theo-damiani, thanks for the explanation!

As we just discussed in the standup, here's the traceback I'm —still— getting (with your patch applied):

> Task :app:connectedDebugAndroidTest
Starting 9 tests on Pixel_3a_API_29_x86(AVD) - 10

com.github.steroidteam.todolist.ItemViewActivityTest > removeTaskButtonWorks[Pixel_3a_API_29_x86(AVD) - 10] FAILED 
  androidx.test.espresso.NoMatchingViewException: No views in hierarchy found matching: with id is <com.github.steroidteam.todolist:id/activity_itemview_itemlist>

Okay, I get the same error.

So I will try to fix it but I think with Pixel 2 API 29, Android Level 10 it works

shoutizix commented 3 years ago

I retried the test using Yago's config Pixel_3a_API_29_x86(AVD) and I get the same error as him. I don't really know why it was working on my side the other day...

YagoGG commented 3 years ago

I have also made some research on this, and the issue was indeed introduced in 3beda9a2dcc29cc7ef60a0a6d55f445cfc3494fd, which is where we introduced the ItemViewActivityTest.removeTaskButtonWorks check.

Reverting the commit fixes the issue:

git revert 3beda9a2dcc29cc7ef60a0a6d55f445cfc3494fd -m 1

But of course we also lose the MVVM refactor which is... not great :(

theo-damiani commented 3 years ago

Hey @YagoGG,

If you want I removed the Alert Dialog in PR #112. The tests work fine I think. So maybe I can all try to run PR #112. And if it is okay we merge it. And so we keep the MVVM pattern, and later if we want alert dialog we can work on it in a new branch, but we are not blocked anymore.

YagoGG commented 3 years ago

Sounds good to me @theo-damiani! I will give it a go right now, and hopefully we'll be able to merge it ASAP. Thanks!

theo-damiani commented 3 years ago

So are you agree to close this PR also ?