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.

Sign up for GitHub

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

[New architecture] Improve viewmodel behaviour #2569

Merged
merged 11 commits into from
Jun 11, 2019
Merged

[New architecture] Improve viewmodel behaviour #2569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d636b6d
Use just one livedata with shares
davigonz Jun 6, 2019
90852cf
Decouple livedatas between get shares operation and other ones
davigonz Jun 7, 2019
47f6d00
Fix unit tests
davigonz Jun 7, 2019
96e4d04
Fix UI tests [WIP]
davigonz Jun 7, 2019
2729988
Fix share creation UI tests using idling resources
davigonz Jun 10, 2019
897f981
Fix UI tests (todo - get rid of Thread.sleep)
davigonz Jun 10, 2019
e510a81
Fix UI tests
davigonz Jun 10, 2019
4bf99ad
Delete unused idling resource
davigonz Jun 10, 2019
b60c4e0
Uncomment deletPublicLink test
davigonz Jun 10, 2019
420413a
Fix addExpirationDate() test, included in share edition suite
davigonz Jun 11, 2019
34d458f
Apply CR changes
davigonz Jun 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion owncloudApp/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ android {

testOptions {
unitTests.returnDefaultValues = true
animationsDisabled = false
animationsDisabled = true
}
}

Expand Down
39 changes: 39 additions & 0 deletions ...p/src/androidTest/java/com/owncloud/android/shares/ui/FragmentVisibilityIdlingResource.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.owncloud.android.shares.ui

import android.view.View
import androidx.fragment.app.Fragment
import androidx.test.espresso.IdlingResource

class FragmentVisibilityIdlingResource(private val fragment: Fragment?, private val expectedVisibility: Int) :
IdlingResource {
private var idle: Boolean = false
private var resourceCallback: IdlingResource.ResourceCallback? = null

init {
this.idle = false
this.resourceCallback = null
}

override fun getName(): String {
return FragmentVisibilityIdlingResource::class.java.simpleName
}

override fun isIdleNow(): Boolean {
if (fragment == null) return false

idle = idle || (fragment.isVisible && expectedVisibility == View.VISIBLE) ||
(!fragment.isVisible && expectedVisibility == View.INVISIBLE)

if (idle) {
if (resourceCallback != null) {
resourceCallback!!.onTransitionToIdle()
}
}

return idle
}

override fun registerIdleTransitionCallback(resourceCallback: IdlingResource.ResourceCallback) {
this.resourceCallback = resourceCallback
}
}
168 changes: 118 additions & 50 deletions ...App/src/androidTest/java/com/owncloud/android/shares/ui/usecases/CreatePublicShareTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import android.accounts.AccountManager
import android.content.Context
import android.content.Intent
import android.os.Parcelable
import android.view.View
import androidx.lifecycle.MutableLiveData
import androidx.test.core.app.ApplicationProvider
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.action.ViewActions
import androidx.test.espresso.IdlingRegistry
import androidx.test.espresso.action.ViewActions.click
import androidx.test.espresso.assertion.ViewAssertions.doesNotExist
import androidx.test.espresso.assertion.ViewAssertions.matches
Expand All @@ -45,6 +46,7 @@ import com.owncloud.android.lib.common.accounts.AccountUtils
import com.owncloud.android.lib.resources.status.CapabilityBooleanType
import com.owncloud.android.lib.resources.status.OwnCloudVersion
import com.owncloud.android.shares.db.OCShare
import com.owncloud.android.shares.ui.FragmentVisibilityIdlingResource
import com.owncloud.android.shares.ui.ShareActivity
import com.owncloud.android.shares.viewmodel.OCShareViewModel
import com.owncloud.android.ui.activity.FileActivity
Expand Down Expand Up @@ -123,37 +125,35 @@ class CreatePublicShareTest {
// obtaining an AccountManager instance
val accountManager = AccountManager.get(targetContext)

Thread(Runnable {
accountManager.addAccountExplicitly(account, "a", null)

// include account version, user, server version and token with the new account
accountManager.setUserData(
account,
AccountUtils.Constants.KEY_OC_VERSION,
OwnCloudVersion("10.2").toString()
)
accountManager.setUserData(
account,
AccountUtils.Constants.KEY_OC_BASE_URL,
"serverUrl:port"
)
accountManager.setUserData(
account,
AccountUtils.Constants.KEY_DISPLAY_NAME,
"admin"
)
accountManager.setUserData(
account,
AccountUtils.Constants.KEY_OC_ACCOUNT_VERSION,
"1"
)

accountManager.setAuthToken(
account,
KEY_AUTH_TOKEN_TYPE,
"AUTH_TOKEN"
)
}).start()
accountManager.addAccountExplicitly(account, "a", null)

// include account version, user, server version and token with the new account
accountManager.setUserData(
account,
AccountUtils.Constants.KEY_OC_VERSION,
OwnCloudVersion("10.2").toString()
)
accountManager.setUserData(
account,
AccountUtils.Constants.KEY_OC_BASE_URL,
"serverUrl:port"
)
accountManager.setUserData(
account,
AccountUtils.Constants.KEY_DISPLAY_NAME,
"admin"
)
accountManager.setUserData(
account,
AccountUtils.Constants.KEY_OC_ACCOUNT_VERSION,
"1"
)

accountManager.setAuthToken(
account,
KEY_AUTH_TOKEN_TYPE,
"AUTH_TOKEN"
)
}
}

Expand Down Expand Up @@ -194,32 +194,56 @@ class CreatePublicShareTest {
loadCapabilitiesSuccessfully()
loadSharesSuccessfully(arrayListOf())

// Create share
onView(withId(R.id.addPublicLinkButton)).perform(click())

// Force Espresso to wait until dialog fragment is visible
val createShareFragmentIdlingResource = registerCreateShareFragmentAsIdlingResource()

val newPublicShare = publicShares[0]
savePublicShare(newPublicShare)

createPublicShareSuccesfully(newPublicShare, arrayListOf(newPublicShare))
// New share properly created
sharesLiveData.postValue(
Resource.success(
arrayListOf(newPublicShare)
)
)

// Check whether the dialog to create the public share has been properly closed
onView(withText(R.string.share_via_link_create_title)).check(doesNotExist())
onView(withText(newPublicShare.name)).check(matches(isDisplayed()))

unregisterCreateShareFragmentAsIdlingResource(createShareFragmentIdlingResource)
}

@Test
fun createPublicShareWithAlreadyExistingShares() {
loadCapabilitiesSuccessfully()

val existingPublicShares = publicShares.take(2) as ArrayList<OCShare>

loadSharesSuccessfully(
existingPublicShares
)

onView(withId(R.id.addPublicLinkButton)).perform(click())

val createShareFragmentIdlingResource = registerCreateShareFragmentAsIdlingResource()

val newPublicShare = publicShares[2]
savePublicShare(newPublicShare)

createPublicShareSuccesfully(newPublicShare, publicShares)
// New share properly created
sharesLiveData.postValue(
Resource.success(
publicShares
)
)

// Check whether the dialog to create the public share has been properly closed
onView(withText(R.string.share_via_link_create_title)).check(doesNotExist())
onView(withText(newPublicShare.name)).check(matches(isDisplayed()))

unregisterCreateShareFragmentAsIdlingResource(createShareFragmentIdlingResource)
Copy link

Choose a reason for hiding this comment

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

why don't you include this statement in a "@after" method??

Copy link
Contributor Author

@davigonz davigonz Jun 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Because the idling resource to unregister is created for each test and not shared between different ones.

Copy link

Choose a reason for hiding this comment

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

"@before" and "@after" are functions are called before and after running one single test.

My idea would be to have your tests as following:

private lateinit var idlingResource: <Your type>

@Before
fun setUp() {
    idlingResource = // call constructor
    //register ...
}

@After  
fun tearDown() {
    // unregister
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I know what you mean, I tried that way but the tests were not working fine. Let me recheck it...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@before
fun setUp() {
idlingResource = // call constructor
//register ...
}

I'm not using this because I register the idling resources in the middle of the tests, after executing other lines of code.

If you see the Expresso idling resources docs , there's two different ways to implement it (see below), I decided to use the second one and that's why I do not use @After method either.

Screenshot 2019-06-11 at 14 27 11

}

@Test
Expand All @@ -230,35 +254,71 @@ class CreatePublicShareTest {
/**
* 1st public share
*/
onView(withId(R.id.addPublicLinkButton)).perform(click())

val createShareFragmentIdlingResource = registerCreateShareFragmentAsIdlingResource()

val newPublicShare1 = publicShares[0]
savePublicShare(newPublicShare1)

createPublicShareSuccesfully(newPublicShare1, arrayListOf(newPublicShare1))
// New share properly created
sharesLiveData.postValue(
Resource.success(
arrayListOf(newPublicShare1)
)
)

// Check whether the dialog to create the public share has been properly closed
onView(withText(R.string.share_via_link_create_title)).check(doesNotExist())
onView(withText(newPublicShare1.name)).check(matches(isDisplayed()))

unregisterCreateShareFragmentAsIdlingResource(createShareFragmentIdlingResource)

/**
* 2nd public share
*/
onView(withId(R.id.addPublicLinkButton)).perform(click())

val create2ndShareFragmentIdlingResource = registerCreateShareFragmentAsIdlingResource()

val newPublicShare2 = publicShares[1]
savePublicShare(newPublicShare2)

createPublicShareSuccesfully(newPublicShare2, publicShares.take(2))
// New share properly created
sharesLiveData.postValue(
Resource.success(
publicShares.take(2)
)
)

// Check whether the dialog to create the public share has been properly closed
onView(withText(R.string.share_via_link_create_title)).check(doesNotExist())
onView(withText(newPublicShare2.name)).check(matches(isDisplayed()))

unregisterCreateShareFragmentAsIdlingResource(create2ndShareFragmentIdlingResource)

/**
* 3rd public share
*/
onView(withId(R.id.addPublicLinkButton)).perform(click())

val create3rdShareFragmentIdlingResource = registerCreateShareFragmentAsIdlingResource()

val newPublicShare3 = publicShares[2]
savePublicShare(newPublicShare3)

createPublicShareSuccesfully(newPublicShare3, publicShares)
// New share properly created
sharesLiveData.postValue(
Resource.success(
publicShares
)
)

// Check whether the dialog to create the public share has been properly closed
onView(withText(R.string.share_via_link_create_title)).check(doesNotExist())
onView(withText(newPublicShare3.name)).check(matches(isDisplayed()))

unregisterCreateShareFragmentAsIdlingResource(create3rdShareFragmentIdlingResource)
}

private fun getOCFileForTesting(name: String = "default") = OCFile("/Photos").apply {
Expand Down Expand Up @@ -286,7 +346,7 @@ class CreatePublicShareTest {
sharesLiveData.postValue(Resource.success(shares))
}

private fun createPublicShareSuccesfully(newShare: OCShare, sharesAfterCreation: List<OCShare>) {
private fun savePublicShare(newShare: OCShare) {
`when`(
ocShareViewModel.insertPublicShareForFile(
1,
Expand All @@ -295,19 +355,27 @@ class CreatePublicShareTest {
-1,
false
)
).thenReturn(sharesLiveData)

// 1. Open dialog to create new public share
onView(withId(R.id.addPublicLinkButton)).perform(click())
).thenReturn(
MutableLiveData<Resource<Unit>>().apply {
postValue(Resource.success())
}
)

// 2. Save share
onView(withId(R.id.saveButton)).perform(click())
}

// 3. New share properly created
sharesLiveData.postValue(
Resource.success(
sharesAfterCreation
)
private fun registerCreateShareFragmentAsIdlingResource(): FragmentVisibilityIdlingResource {
val idlingResource = FragmentVisibilityIdlingResource(
activityRule.activity.supportFragmentManager.findFragmentByTag(ShareActivity.TAG_PUBLIC_SHARE_DIALOG_FRAGMENT),
View.VISIBLE
)
IdlingRegistry.getInstance().register(idlingResource)
return idlingResource
}

private fun unregisterCreateShareFragmentAsIdlingResource(
fragmentVisibilityIdlingResource: FragmentVisibilityIdlingResource
) {
IdlingRegistry.getInstance().unregister(fragmentVisibilityIdlingResource)
}
}
Loading