mirror of
https://github.com/mattermost/mattermost-mobile.git
synced 2026-11-11 05:30:12 +00:00
* test(e2e): add migrated Detox and Maestro multi-device suites New-test slice of the #9893 split (3 of 3). Adds the migrated Detox suites (about, set status, user attributes, autocomplete search, unarchive channel, message char limit, save/unsave message, team switching), the Maestro multi-device channel-sync flows, and only the page-object helpers and app testID fix those suites need. * test(e2e): align new suite locations and clean up review nits - Move set_status into products/channels/account and team_switching into products/channels/teams so the new suites match the existing layout (products/channels held only subdirectories before). - Name the unarchive_channel server "Server 2" to match the site it connects to. - Drop an unused catch binding and a stale scroll comment in about.e2e. Co-authored-by: Cursor <cursoragent@cursor.com> * test(e2e): address CodeRabbit review on migration suites Harden alert dismissal, user-attributes helpers, message-limit assertion, and Maestro two-device channel sync polling/script wiring. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(e2e): fix the two iOS flakes in this PR's run (MM-T216_1, MM-T4863_2) Both diagnosed from the run-30652316213 artifacts, not guessed. Neither test is touched by this PR's migration work; they surfaced in its iOS job and both have a real cause rather than pure environment noise. MM-T216_1 (post_display_behavior) — "View <PasteInput_RCTUITextView> is not visible: does not pass visibility percent threshold (100)" when sending. testStart.png shows a clean channel list; testFnFailure.png shows the post-options bottom sheet open over the draft input. Nothing between them but `scroll(5000, 'up', 0.5, 0.5)`, so that single long synthetic drag dwelled on the post under its start point long enough for the post's long-press handler to fire. Every other post-list scroll in the suite uses 50-300px. - Scroll in 500px increments instead, breaking when Detox reports the content edge (it throws when a scroll begins at the edge, so a fixed increment count over a 20-post channel would otherwise fail reliably). - Probe-and-close the sheet if a gesture still trips a long press, before the new assertion, so an open sheet cannot satisfy it by occlusion. - Assert the list actually left the bottom, so the scroll-to-bottom check is not vacuously true when the gesture does nothing. MM-T4863_2 (follow_and_unfollow_messsage) — timed out at TIMEOUT(2s) waiting for post_footer.following_thread.button; the failure screenshot shows no thread footer on the post at all. Its sibling MM-T4863_1 performs the identical follow and passes, because it carries two guards this test omits: a documented wait for CRT's asynchronous thread creation before opening post options, and waitForElementToBeVisible for the footer rather than a 2s budget. TWO_SEC here was the tightest budget in the file and the only network-dependent assertion given less time than the local ones around it (FOUR_SEC). - Add the same CRT thread-creation wait before opening post options. - Use waitForElementToBeVisible for the Following button, and TEN_SEC for the unfollow assertion, matching MM-T4863_1. This is not "raise the timeout until green": both changes align these tests with patterns already proven in the same file and suite. Verified: cd detox && npm run tsc && npm run lint both clean. Not run on a device — these reproduce only under CI gesture/load timing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(e2e): address CodeRabbit review feedback that is still valid Verified all 19 inline comments against the current branch. 16 were already fixed here (CodeRabbit reviewed an earlier snapshot) or are non-issues; two needed work. post_display_behavior.e2e.ts (comment 3693155180) — both points were right: - Dropped the generic catch around the scroll loop so real Detox/bridge/gesture failures propagate. Rather than catching Detox's edge error, the need for it is gone: two 500px scrolls leave the bottom while staying inside the content of a 20-post channel, so no scroll starts at the edge. - Moved PostOptionsScreen.close() out of the probe's catch, so a failure closing the sheet is no longer swallowed as "no sheet present". user_attributes.e2e.ts (comment 3666329915) — took the reviewer's documented alternative. Per-test provisioning would mean re-polling an instance-wide feature flag and re-creating instance-scoped field definitions for every test, so the deviation from "no shared mutable state between it() blocks" stays, with an explicit note on why and what a new test in this suite must not assume. Not changed, with reasons: - follow_and_unfollow_messsage.e2e.ts (3693155177): the cited pattern at mm_blocks_test_helper.ts:350-388 is a deadline-bounded longPress retry, not a thread-state poll, so it supplies no state to wait on. The run evidence also points elsewhere: the failure was at the footer assertion, not at waitFor(followThreadOption), so the follow option was present and tapped. The fixed wait is parity with the passing MM-T4863_1 and the assertions after it are already bounded. - save_option.tsx: savePreference/deleteSavedPost already log via logDebug(getFullErrorMessage(error)) and return {error}, so a component-level logError would double-log the same failure into Sentry with no user-facing change. The real gap is user-visible feedback, which that suggestion does not add either. Verified: cd detox && npm run tsc && npm run lint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(e2e): fix MM-T216_1 on Android — scroll until the post leaves the viewport Diagnosed from the run-30688183414 machine-16 artifacts, not guessed. The test failed on Android at SHA7e2dc91while passing on iOS: Test Failed: 'not (view has effective visibility <VISIBLE> and view.getGlobalVisibleRect() covers at least <50> percent of the view's area)' Got: ReactViewGroup{... 0,0-1440,91 ... tag=channel.post_list.post.rig7wn9aztyn3pnoxyoyniwrqe} at post_display_behavior.e2e.ts:124 → expect(lastFillerItem).not.toBeVisible() The failing assertion is the one added in9b8d1df, and it encodes a viewport assumption rather than a product expectation. After 2 x scroll(500,'up') the newest post has moved from the bottom to the very top of the screen — the scroll did work — but the post is only 91px tall in a ~2400px emulator viewport, so it still covers well over 50% of its own area and remains "visible". 1000px of scroll clears an iOS viewport and does not clear an Android one. The behaviour under test (sending a message scrolls the list back to the bottom) is unaffected. - Replace the two fixed 500px scrolls with waitFor(lastFillerItem).not.toBeVisible().whileElement(flat_list).scroll(300,'up'), so the loop terminates exactly when the newest post has left the viewport on either platform instead of on a hardcoded pixel budget. - Keep 300px increments: each synthetic drag stays too short to dwell on a post and fire its long-press handler, which is what the original single 5000px drag did (opening the post-options sheet over the draft input). - Keep the probe-and-close of the post-options sheet and the re-assertion at line 124, so the scroll-to-bottom check still cannot pass vacuously — neither when the gesture does nothing nor when a sheet, not the scroll, hides the post. This is not "raise the timeout until green": whileElement + the post-list flat_list is already the idiom used by interactive_dialog_plugin.e2e.ts:637 and theme_color_picker.e2e.ts:109, and .not chaining is type-supported (Expect<R>.not: this -> WaitFor.whileElement in detox.d.ts). Blast radius: one it() block; no page objects or shared helpers touched. Not run on a device — this reproduces only under CI viewport/gesture timing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(e2e): adopt and enforce # # / # * comment markers in Maestro flows Review feedback: Maestro yml files had no standard comment convention, so they could not be mapped onto the test management system the way the Detox specs can. Adopt the Detox convention verbatim — `# #` for a step, `# *` for an assertion — across every flow rather than only the two added by this PR, so the convention is actually in force instead of merely documented. - Convert all 40 files (21 flows + 18 subflows + preflight/_connect_check.yml), replacing the ad-hoc `# --- X ---` and `# ── X ──` section banners. Only the first line of a block carries the marker; continuation lines stay plain `#`, matching Detox. Markers sit at the block's own indentation, including inside nested `commands:` lists. - Document the convention in GUIDELINES.md as a new §2 subsection with a DO/BAD example, plus an authoring-checklist item. - Enforce it in scripts/validate-flow-headers.sh: any comment block that introduces a command must carry a marker. Comment blocks introducing no command (trailing notes) are exempt, and unlike the Zephyr header contract this check also covers subflows/**, preflight/** and _-prefixed fragments. Runs in CI via the existing "Validate Maestro flow headers" job. Also: state in the six flows/multi_device/* headers that they are NOT RUN IN CI. Review feedback asked why the MM-T837 flows are missing from the iOS and Android reports. They are excluded by design — run_ci_batches.sh skips /multi_device/ because CI provisions one device per job, and README lists these as two-device manual — but a flow file that runs nowhere reads as dead, so each header now names the exclusion and the script that runs it. Verified: - validate-flow-headers.sh caught two nested blocks missed by hand; after fixing those it reports "all 21 flow file(s) passed". - Negative test confirmed the new check reaches subflows: un-marking one line in subflows/waits/wait_for_home_tab.yml produced the expected error. - All 40 files parse as YAML (header and body documents). - Comment-only proof: diffing every file's non-comment, non-blank lines against the pre-change revision reports no differences in any file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(e2e): correct inaccurate flow comments and the iOS logout guard Review feedback on the comment-marker pass. Five comment corrections plus one real latent bug found while verifying them. - channel_bookmark_file.yml: the modal-close wait said "if upload was still completing when Save was tapped", but the flow waits for channel_bookmark.file.upload_complete BEFORE tapping Save, so that can't happen. Describe it as waiting for the save + navigation pop, and leave bookmark-creation verification to the channel-info list assertion below. - channel_bookmark_link_external.yml: the block starting with longPressOn was marked # * though its first command is an action. Split into # # for the long-press and # * for the options-sheet assertion. - file_type_preview.yml: the PDF-open wait was marked # * but carries optional: true, so it asserts nothing. Re-mark it # # and record why it cannot become an assertion yet: app/hooks/files.ts opens either Screens.PDF_VIEWER (which has no testID at all) or the OS viewer via viewDocument(), depending on EnableSecureFilePreview. - login.yml: the shared "Allow" tap was described as iOS-only although it runs on both platforms and also catches the Android dialog when it surfaces after navigation cleanup. - logout.yml: dropped the claim that both horizontal and vertical iOS alert layouts are tried — only the horizontal 70%,57% position is tapped. - logout.yml: fix the logout-completion guard, which was vacuously true on iOS. login_form.username.input is a FloatingTextInput (app/screens/login/ form.tsx:372), whose testID is not queryable by id: on iOS per GUIDELINES §3, so `notVisible` always matched there and a logout that landed on the login form instead of the server list would have timed out waiting for server_list.screen. iOS now anchors on the RNEButton login_form.forgot_password.button (form.tsx:403), the same anchor login.yml uses; Android keeps username.input. Both still accept either destination. - assert_timezone_region_local.yml: the marker and the file header both claimed a non-empty region label while the flow only asserts "Off" is absent. Maestro has no non-empty-text primitive, so correct the claim rather than ship an unverified regex selector. CI passes EXPECTED_TIMEZONE_REGION and uses assert_timezone_region_label.yml, which does assert the region text.
138 lines
4.4 KiB
Bash
Executable File
138 lines
4.4 KiB
Bash
Executable File
#!/usr/bin/env bash
|
|
# Validates detox/maestro/flows/**/*.yml against the header contract in GUIDELINES.md,
|
|
# and every flow/subflow/preflight body against the step/assertion comment convention.
|
|
# Run from repo root: bash detox/maestro/scripts/validate-flow-headers.sh
|
|
set -euo pipefail
|
|
|
|
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
|
MAESTRO_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)"
|
|
FLOWS_DIR="${MAESTRO_DIR}/flows"
|
|
|
|
if [[ ! -d "${FLOWS_DIR}" ]]; then
|
|
echo "::error::Expected flows directory at ${FLOWS_DIR}"
|
|
exit 1
|
|
fi
|
|
|
|
failures=0
|
|
checked=0
|
|
|
|
|
|
fail() {
|
|
local file="$1"
|
|
local msg="$2"
|
|
echo "::error file=${file}::${msg}"
|
|
failures=$((failures + 1))
|
|
}
|
|
|
|
extract_header() {
|
|
local file="$1"
|
|
awk '
|
|
/^---[[:space:]]*$/ { exit }
|
|
{ print }
|
|
' "${file}"
|
|
}
|
|
|
|
validate_flow() {
|
|
local file="$1"
|
|
local rel="${file#${MAESTRO_DIR}/}"
|
|
local header
|
|
header="$(extract_header "${file}")"
|
|
checked=$((checked + 1))
|
|
|
|
# Ticket line: # MM-T1234: or # MM-T67856_1:
|
|
local ticket
|
|
ticket="$(printf '%s\n' "${header}" | grep -E '^# MM-T[0-9]+(_[0-9]+)?(\([^)]*\))?:' | head -1 | sed -E 's/^# (MM-T[0-9]+(_[0-9]+)?).*/\1/' || true)"
|
|
if [[ -z "${ticket}" ]]; then
|
|
fail "${rel}" "Missing ticket line (# MM-TXXXX: <behavior summary>)"
|
|
return
|
|
fi
|
|
|
|
for section in 'PRE-CONDITIONS:' 'REQUIRED ENV VARS:' 'ASSERTIONS:' 'testIDs:'; do
|
|
if ! printf '%s\n' "${header}" | grep -qF "# ${section}"; then
|
|
fail "${rel}" "Missing header section: # ${section}"
|
|
fi
|
|
done
|
|
|
|
if ! printf '%s\n' "${header}" | grep -qE '^appId:[[:space:]]*\$\{MAESTRO_APP_ID\}[[:space:]]*$'; then
|
|
fail "${rel}" 'appId must be exactly: appId: ${MAESTRO_APP_ID}'
|
|
fi
|
|
|
|
if ! printf '%s\n' "${header}" | grep -qE "^[[:space:]]*-[[:space:]]+${ticket}[[:space:]]*$"; then
|
|
fail "${rel}" "tags must include Zephyr id ${ticket} (tags: [${ticket}])"
|
|
fi
|
|
|
|
# testIDs section should list at least one entry (or an explicit none/system note).
|
|
local testids_block
|
|
testids_block="$(printf '%s\n' "${header}" | awk '
|
|
/^# testIDs:/ { found=1; next }
|
|
found && /^# [A-Z]/ && !/^# testIDs:/ { exit }
|
|
found { print }
|
|
')"
|
|
if ! printf '%s\n' "${testids_block}" | grep -qE '^#[[:space:]]+-'; then
|
|
fail "${rel}" 'testIDs section must list at least one entry (# - ...)'
|
|
fi
|
|
|
|
|
|
}
|
|
|
|
# Every comment block that introduces one or more commands must start with the step/
|
|
# assertion marker from GUIDELINES.md §2 — `# #` for a step, `# *` for an assertion —
|
|
# mirroring the Detox `// #` / `// *` convention so both suites map onto the same test
|
|
# management system. Continuation lines stay plain `#`. Comment blocks that introduce no
|
|
# command (trailing notes) are exempt.
|
|
validate_body_comments() {
|
|
local file="$1"
|
|
local rel="${file#${MAESTRO_DIR}/}"
|
|
local bad_lines
|
|
bad_lines="$(awk '
|
|
BEGIN { body = 0; n = 0 }
|
|
body == 0 {
|
|
if ($0 ~ /^---[[:space:]]*$/) { body = 1 }
|
|
next
|
|
}
|
|
{ n++; line[n] = $0; lineno[n] = FNR }
|
|
END {
|
|
i = 1
|
|
while (i <= n) {
|
|
if (line[i] !~ /^[[:space:]]*#/) { i++; continue }
|
|
start = i
|
|
j = i
|
|
while (j <= n && line[j] ~ /^[[:space:]]*#/) { j++ }
|
|
k = j
|
|
while (k <= n && line[k] ~ /^[[:space:]]*$/) { k++ }
|
|
if (k <= n && line[k] ~ /^[[:space:]]*-/ && line[start] !~ /^[[:space:]]*# [#*] /) {
|
|
print lineno[start]
|
|
}
|
|
i = j
|
|
}
|
|
}
|
|
' "${file}")"
|
|
|
|
local lineno
|
|
for lineno in ${bad_lines}; do
|
|
echo "::error file=${rel},line=${lineno}::Comment block introducing a command must start with '# #' (step) or '# *' (assertion) — see GUIDELINES.md §2"
|
|
failures=$((failures + 1))
|
|
done
|
|
}
|
|
|
|
while IFS= read -r -d '' flow; do
|
|
base="$(basename "${flow}")"
|
|
[[ "${base}" == _* ]] && continue
|
|
validate_flow "${flow}"
|
|
done < <(find "${FLOWS_DIR}" -name '*.yml' -type f -print0 | sort -z)
|
|
|
|
# The comment convention applies to every flow file, including the `_`-prefixed and
|
|
# subflow/preflight fragments that are exempt from the Zephyr header contract.
|
|
while IFS= read -r -d '' flow; do
|
|
validate_body_comments "${flow}"
|
|
done < <(find "${FLOWS_DIR}" "${MAESTRO_DIR}/subflows" "${MAESTRO_DIR}/preflight" -name '*.yml' -type f -print0 2>/dev/null | sort -z)
|
|
|
|
if [[ "${failures}" -gt 0 ]]; then
|
|
echo ""
|
|
echo "validate-flow-headers: ${failures} failure(s) in ${checked} flow file(s)."
|
|
echo "See detox/maestro/GUIDELINES.md §2 and flows/account/attach_logs.yml for the canonical header."
|
|
exit 1
|
|
fi
|
|
|
|
echo "validate-flow-headers: all ${checked} flow file(s) passed."
|