[Feature/roborazzi]: Screenshot Test 작업 추가 #323
Conversation
Test Results20 tests 20 ✅ 18s ⏱️ Results for commit 234d17a. ♻️ This comment has been updated with latest results. |
|
@KwakEuiJin CI 실패하는 부분 수정 먼저부탁드려요. |
| @@ -56,4 +56,4 @@ jobs: | |||
| uses: actions/upload-artifact@v3 | |||
| with: | |||
| name: Test Results | |||
| path: "**/test-results/**/*.xml" | |||
| path: "**/test-results/**/*.xml" | |||
app/build.gradle.kts
Outdated
| } | ||
|
|
||
| testImplementation(projects.core.testing) | ||
| } |
| runCatching { | ||
| coroutineScope { | ||
| testBody() | ||
| } |
There was a problem hiding this comment.
runTest가 이미 코루틴인것 같은데 coroutineScope{}을 추가로 해야할 이유가 있나요?
There was a problem hiding this comment.
단편적으로는 testBody()의 예외를 한번에 처리하고자하였지만 coroutineScope를 추가하지 않아도 이를 구현할 수 있는 방법이 있을 것 같습니다! 해당 부분 아래와 같이 수정해두겠습니다.
fun runTestWithLogging(
context: CoroutineContext = EmptyCoroutineContext,
timeout: Duration = 30.seconds,
testBody: suspend TestScope.() -> Unit,
) = runTest(context, timeout) {
runCatching {
testBody()
}.onFailure { exception ->
exception.printStackTrace()
throw exception
}
}
There was a problem hiding this comment.
runTest가 이미 coroutine이고, runCatch {}를 이용해 잡을 수 있습니다. coroutineScop은 어차피 withContext와 동일하게 블러킹 후에 응답을 주는것일 뿐이라 위 코드에서는 의미없는 코드에 해당합니다. runTest가 이미 한줄한줄 suspend 하기 위함이니
| throw exception | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
runCatch{} 문법에 맞는 형태가 좋을것 같네요
.onFai... onSu...
| import dagger.hilt.android.AndroidEntryPoint | ||
| import kotlinx.coroutines.flow.MutableStateFlow | ||
|
|
||
| @AndroidEntryPoint | ||
| class MainActivity : AppCompatActivity() { | ||
| class MainActivity : ComponentActivity() { |
There was a problem hiding this comment.
Q) AppCompatActivity 에서 ComponentActivity 로 변경한 이유가있나요?
There was a problem hiding this comment.
AndroidComposeTestRule를 통해 ComponentActivity를 통해 컴포즈 UI를 테스트할 수 있는 환경을 제공하는데요, 특히 Roborazzi에서는 onNode를 통해 Semantics 노드에 접근해서 root 화면을 찾거나 setContent 블럭을 활용하여 Composable을 Test 할 수 있도록 하기 때문입니다!
@OptIn(ExperimentalTestApi::class)
class AndroidComposeTestRule<R : TestRule, A : ComponentActivity> private constructor(
val activityRule: R,
private val environment: AndroidComposeUiTestEnvironment<A>
) : ComposeContentTestRule
There was a problem hiding this comment.
AppCompatActivity가 상속 구조상 ComponentActivity를 상속 받고있는데... AppCompoactActivity가 필요한 경우라면 Roborazzi를 사용하지 못하는건 아닐것 같네요.
There was a problem hiding this comment.
오 네넵 상속 구조상 가능은 할 것 같습니다만 코틀린 제네릭 타입에서는 upperBounds가 오로지 subType만을 요구한다고 알고 있습니다.
따라서 AndroidComposeTestRule을 사용한다는 가정하에 FragementActivity혹은 ComponentActivity로 캐스팅 하지 않는 이상 힘든 구조일 것으로 예상됩니다.
There was a problem hiding this comment.
Roborazzi을 위한 테스트 앱 모듈이 별도로 존재해야하는건가요?
저도 처음 탑재해보는 ScreenShotTest 플러그인이여서 DroidKaigi를 참고하여 개발하였습니다! 위 프로젝트를 기반으로 답변드리자면 Roborazzi를 위해서 테스트 앱 모듈이 별도로 존재할 필요는 없습니다.
각 Feature Module별로 필요한 Robot을 만들고 이를 Feature내의 Test파일에서 해당 Robot을 주입받아 필요한 Composable, 혹은 Screen을 capture하여 이전 화면과 비교하는 것으로 파악했습니다.
또한 compare-screenshot-comment CI의 경우 main에 머지가 된 이후에 동작할 수 있기 때문에 개인 레포지토리에서 테스트한 링크를 첨부드립니다.
kez-lab#4
마지막으로 ScreenShotTest의 필요도가 어떤 부분까지인지 방향성을 알고싶습니다. 현재는 앱 최상위 루트의 화면만을 테스트중인데요.
세부적인 화면의 테스트도 필요한지 궁금합니다.
| runCatching { | ||
| coroutineScope { | ||
| testBody() | ||
| } |
There was a problem hiding this comment.
단편적으로는 testBody()의 예외를 한번에 처리하고자하였지만 coroutineScope를 추가하지 않아도 이를 구현할 수 있는 방법이 있을 것 같습니다! 해당 부분 아래와 같이 수정해두겠습니다.
fun runTestWithLogging(
context: CoroutineContext = EmptyCoroutineContext,
timeout: Duration = 30.seconds,
testBody: suspend TestScope.() -> Unit,
) = runTest(context, timeout) {
runCatching {
testBody()
}.onFailure { exception ->
exception.printStackTrace()
throw exception
}
}
| throw exception | ||
| } | ||
| } | ||
| } |
| import dagger.hilt.android.AndroidEntryPoint | ||
| import kotlinx.coroutines.flow.MutableStateFlow | ||
|
|
||
| @AndroidEntryPoint | ||
| class MainActivity : AppCompatActivity() { | ||
| class MainActivity : ComponentActivity() { |
There was a problem hiding this comment.
AndroidComposeTestRule를 통해 ComponentActivity를 통해 컴포즈 UI를 테스트할 수 있는 환경을 제공하는데요, 특히 Roborazzi에서는 onNode를 통해 Semantics 노드에 접근해서 root 화면을 찾거나 setContent 블럭을 활용하여 Composable을 Test 할 수 있도록 하기 때문입니다!
@OptIn(ExperimentalTestApi::class)
class AndroidComposeTestRule<R : TestRule, A : ComponentActivity> private constructor(
val activityRule: R,
private val environment: AndroidComposeUiTestEnvironment<A>
) : ComposeContentTestRule
There was a problem hiding this comment.
이 Activity 따로 추가하신 이유가 있을까요? 이전 PR에 이와 같은 역할을 가진 Activity를 추가 했습니다.
There was a problem hiding this comment.
HiltTestActivity를 추가한 이유는 다음과 같습니다.
현재는 app module에서 MainActivity를 활용하여 테스트가 진행됩니다. 하지만 추후 Feature모듈에서 AndroidComposeTestRule을 활용하여 Test를 진행할 때는 MainActivity를 참조하지 못하기 때문에 HiltTestActivity를 추가하여 이를 가능하게 했습니다.
다만 이미 동일한 역할을 가진 Activity가 있는 줄 인지하지 못했습니다 ㅎㅎ 해당 부분 삭제 진행해두겠습니다!
There was a problem hiding this comment.
추후 추가될 수 있는 feature 모듈용 테스트가 필요하다면 이에 대한 테스트용 모듈을 따로 배치하는게 적합합니다. 현재는 App 모듈 하나 뿐이니 의미가 없는 코드이구요.
Issue
Overview (Required)
Links
https://github.com/takahirom/roborazzi
https://github.com/takahirom/roborazzi-compare-on-github-comment-sample
https://github.com/DroidKaigi/conference-app-2023
Screenshot