[Refactor] Maestro QA Build로 전환 및 auth flow 작성#1506
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAndroid Maestro CI 파이프라인을 QA 빌드 경로로 전환하고, 모든 Maestro YAML의 APP_ID를 QA 패키지명으로 업데이트했습니다. 학생 인증 스모크 스위트와 8개의 신규 학생 플로우가 추가됐으며, 비밀번호 필드 및 콜밴 아이콘 접근성이 개선됐습니다. ChangesQA 빌드 전환 및 학생 인증 Maestro 스위트
Android 접근성 개선
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 분 Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/run_maestro_suite.sh (1)
48-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick win인증 스위트가 아닌 실행에도 비밀번호 시크릿이 전달됩니다.
Line 52에서
USER_PASSWORD를 항상 주입하고 있어 guest 스위트에도 인증 비밀번호가 노출됩니다. 인증 스위트에서만 전달되도록 분기해 주세요.패치 제안
record_args=( --local --debug-output "$debug_output" -e "APP_ID=$app_id" - -e "USER_PASSWORD=${USER_PASSWORD:-}" "$wrapper_flow" "$video_output" ) + +if [[ "$suite_name" == *auth* ]]; then + record_args+=(-e "USER_PASSWORD=$USER_PASSWORD") +fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/run_maestro_suite.sh around lines 48 - 53, The record_args array in the run_maestro_suite.sh script unconditionally passes the USER_PASSWORD environment variable to all test suites, including non-authentication suites like guest suites, which is a security concern. Modify the script to conditionally include the -e "USER_PASSWORD=${USER_PASSWORD:-}" argument only when running authentication suites. Add a conditional check before appending this environment variable to the record_args array, ensuring that the USER_PASSWORD secret is only injected for authentication suite executions and not for guest or other non-authentication suite runs.
🧹 Nitpick comments (1)
.maestro/guest/auth_sign_in.yaml (1)
32-33: ⚡ Quick win입력값 문자열 자체를 탭 타깃으로 쓰지 않도록 고정해 주세요.
Line 32처럼
"invalid-user"텍스트를 직접 탭하면 입력 포맷/자동 정리 동작이 바뀔 때 시나리오가 쉽게 깨집니다. 필드 라벨(또는 가능하면 리소스 id) 기준으로 포커스 후eraseText하는 방식이 더 안정적입니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.maestro/guest/auth_sign_in.yaml around lines 32 - 33, The tapOn action at line 32 is using the literal input value "invalid-user" as the tap target, which is brittle and will break when input format or validation behavior changes. Replace the tapOn action to target the input field by its field label or resource ID instead of the text value itself, then keep the eraseText action to clear the field. This approach provides more stable targeting that is independent of the actual input values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.maestro/student/settings_logged_in.yaml:
- Line 23: The assertVisible check on line 23 uses a hardcoded test account
identifier "androidqa" which makes the test brittle and will break CI whenever
the test account changes. Replace this hardcoded account-specific assertion with
a more robust, account-independent validation that checks for the presence of an
ID label or field element instead, so the test remains valid regardless of which
test account is used.
In
`@feature/user/src/main/java/in/koreatech/koin/feature/user/component/KoinUserPasswordTextField.kt`:
- Around line 61-66: The `setText` semantic action in the
KoinUserPasswordTextField class is calling `onValueChange(text.text)` directly,
which bypasses the input normalization logic (maxLength and trim() rules)
defined in lines 68-73. To fix this, apply the same normalization rules within
the `setText` block before calling onValueChange. Extract or reuse the
normalization logic (maxLength constraint and trim operation) that is applied in
the regular input handler and apply it to the text parameter within the setText
lambda so that both the semantic input path and regular input path follow the
same validation and formatting rules.
---
Outside diff comments:
In @.github/scripts/run_maestro_suite.sh:
- Around line 48-53: The record_args array in the run_maestro_suite.sh script
unconditionally passes the USER_PASSWORD environment variable to all test
suites, including non-authentication suites like guest suites, which is a
security concern. Modify the script to conditionally include the -e
"USER_PASSWORD=${USER_PASSWORD:-}" argument only when running authentication
suites. Add a conditional check before appending this environment variable to
the record_args array, ensuring that the USER_PASSWORD secret is only injected
for authentication suite executions and not for guest or other
non-authentication suite runs.
---
Nitpick comments:
In @.maestro/guest/auth_sign_in.yaml:
- Around line 32-33: The tapOn action at line 32 is using the literal input
value "invalid-user" as the tap target, which is brittle and will break when
input format or validation behavior changes. Replace the tapOn action to target
the input field by its field label or resource ID instead of the text value
itself, then keep the eraseText action to clear the field. This approach
provides more stable targeting that is independent of the actual input values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f92e8c56-61eb-454d-9e30-a97231defd2e
📒 Files selected for processing (38)
.github/scripts/run_maestro_suite.sh.github/workflows/maestro.yml.maestro/common/dismiss_banner.yaml.maestro/common/launch_app.yaml.maestro/common/login.yaml.maestro/common/logout.yaml.maestro/common/open_drawer.yaml.maestro/guest/article.yaml.maestro/guest/auth_find_id.yaml.maestro/guest/auth_find_password.yaml.maestro/guest/auth_sign_in.yaml.maestro/guest/auth_sign_up.yaml.maestro/guest/banner.yaml.maestro/guest/bus.yaml.maestro/guest/callvan.yaml.maestro/guest/club.yaml.maestro/guest/dining.yaml.maestro/guest/home.yaml.maestro/guest/land.yaml.maestro/guest/lost_and_found.yaml.maestro/guest/operating_info.yaml.maestro/guest/settings.yaml.maestro/guest/store.yaml.maestro/guest/timetable.yaml.maestro/student/article_keyword.yaml.maestro/student/callvan_create.yaml.maestro/student/callvan_detail.yaml.maestro/student/callvan_notifications.yaml.maestro/student/chat_list.yaml.maestro/student/lostandfound_keyword.yaml.maestro/student/lostandfound_write.yaml.maestro/student/settings_logged_in.yaml.maestro/suites/student_auth_smoke.yaml.maestro/suites/student_guest_smoke.yamlfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/detail/CallvanDetailScreen.ktfeature/callvan/src/main/java/in/koreatech/koin/feature/callvan/ui/list/CallvanListScreen.ktfeature/user/src/main/java/in/koreatech/koin/feature/user/component/KoinUserPasswordTextField.ktfeature/user/src/main/res/values/strings.xml
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
bcsd-b-bot
Bot
added
ci:passed
and removed
ci:passed
coderabbit:complete
claude:complete
labels
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/run_maestro_suite.sh:
- Around line 14-17: The USER_PASSWORD validation in the conditional block
starting at line 14 is unconditionally required for all Maestro suites, but it
should only be enforced for suites that actually require authentication. The
check should be made conditional based on whether the test suite being run is an
authentication-required suite or a guest suite that doesn't require login
credentials. Move the USER_PASSWORD validation to only execute when running
auth-required suites, allowing guest suites without authentication steps to run
without requiring this password to be set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3e43719-6483-4385-b69c-e6c3781c2ba1
📒 Files selected for processing (7)
.github/scripts/run_maestro_suite.sh.github/workflows/maestro.yml.maestro/guest/auth_sign_in.yaml.maestro/guest/dining.yaml.maestro/student/article_keyword.yaml.maestro/student/lostandfound_keyword.yaml.maestro/suites/student_smoke.yaml
💤 Files with no reviewable changes (2)
- .maestro/student/lostandfound_keyword.yaml
- .maestro/student/article_keyword.yaml
✅ Files skipped from review due to trivial changes (1)
- .maestro/suites/student_smoke.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- .maestro/guest/dining.yaml
|
KYM-P
approved these changes
Jun 19, 2026
KYM-P
left a comment
KYM-P
left a comment
Collaborator
There was a problem hiding this comment.
GOOD
아이디 부분이 고정되어있는 것이 좀 걸리지만
qa 로 다계정 flow 를 할게 아니라면 문제는 없어 보이긴 하네요
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 개요
PR 체크리스트
작업사항
작업사항의 상세한 설명
논의 사항
스크린샷
추가내용
Summary by CodeRabbit
릴리스 노트
Tests
Chores
Accessibility