Skip to content

[build-tools] Fix ai-*.json is not guarenteed in Maestro 2.5#3639

Open
hSATAC wants to merge 3 commits intomainfrom
ash/eng-19927-fix-ai-json-not-guarenteed
Open

[build-tools] Fix ai-*.json is not guarenteed in Maestro 2.5#3639
hSATAC wants to merge 3 commits intomainfrom
ash/eng-19927-fix-ai-json-not-guarenteed

Conversation

@hSATAC
Copy link
Copy Markdown
Contributor

@hSATAC hSATAC commented Apr 28, 2026

Why

Maestro 2.5.0 (released 2026-04-27) changed TestDebugReporter.saveSuggestions to early-return when a flow has no AI screen outputs. As a result, ai-${flow_name}.json is now only emitted for flows using AI commands (e.g. assertWithAI).

Two callers in @expo/build-tools relied on ai-*.json for the flow_name → flow_file_path mapping:

  • Smart retry (parseFailedFlowsFromJUnit): lookup misses every time on 2.5.0+, so smart retry silently degrades to dumb retry — a functional regression of [build-tools] Add eas/run_maestro_tests step with smart retry #3635 against the latest Maestro.
  • Test result reporting (parseMaestroResults): the path on uploaded testCaseResults falls back to flow name. Cosmetic-only — path is read in the workflow detail UI as a hover tooltip.

Linear: ENG-19927. Stacked on #3635.

How

Replace the ai-*.json walk with a self-built flow_name → flow_file_path map (buildFlowNameToPathMap). The helper walks the user's flow_paths input, parses each flow YAML's top-level name (or falls back to file basename without extension), and produces a name → path map. Both parseFailedFlowsFromJUnit and parseMaestroResults consume it.

  • Map values are project-root-relative paths (preserving today's GraphQL path semantics).
  • Duplicate flow names produce a warning and disable smart retry for that run; consumers receive null and fall back to the previous degraded behavior.
  • Adds a new optional flow_paths input to eas/report_maestro_test_results. Universe-side wiring (passing job.params.flow_path into this input) is a separate PR.

Behavior change for eas/report_maestro_test_results: single-attempt retryCount is no longer derived from ai-*.json occurrence count, falls back to 0. Production impact: zero — no readers of retryCount, and the only caller of the single-attempt branch (the bash step) is retired by #3641.

Backward compatibility during deployment: between this PR landing and the universe wiring landing, old job specs (without flow_paths on the report step) run against the new code. The step silently falls back to null map → flow-name-as-path, no warnings.

Test Plan

  • 27 new unit tests in maestroFlowDiscovery.test.ts covering: top-level name, basename fallback, malformed YAML, missing files, leading --- document marker, directory walks, symlink cycles (real fs), workspace config.yaml skip, duplicate-name handling, same-file dedup, absolute paths.
  • All existing tests updated (88 passing across maestroResultParser.test.ts, reportMaestroTestResults.test.ts, runMaestroTests.test.ts).
  • Local end-to-end verification on Maestro 2.5.0 against the playground (intentionally flaky flows):
    • --shard-split=2: failed flows from attempt 1 retried with project-relative paths; sharding adjusted to the retried subset.
    • No sharding: failed flows retried in attempt 2; flaky failures recovered (final PASS).

@linear
Copy link
Copy Markdown

linear Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

hSATAC commented Apr 28, 2026

@hSATAC hSATAC marked this pull request as ready for review April 28, 2026 13:40
@github-actions
Copy link
Copy Markdown

Subscribed to pull request

File Patterns Mentions
**/* @douglowder

Generated by CodeMention

@hSATAC hSATAC added the no changelog PR that doesn't require a changelog entry label Apr 28, 2026
@hSATAC hSATAC marked this pull request as draft April 28, 2026 13:40
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2026

Codecov Report

❌ Patch coverage is 91.09589% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.63%. Comparing base (4da5b40) to head (0755678).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...-tools/src/steps/functions/maestroFlowDiscovery.ts 89.82% 11 Missing ⚠️
...ls/src/steps/functions/reportMaestroTestResults.ts 83.34% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3639      +/-   ##
==========================================
+ Coverage   56.62%   56.63%   +0.01%     
==========================================
  Files         886      888       +2     
  Lines       38331    38416      +85     
  Branches     7977     8009      +32     
==========================================
+ Hits        21703    21754      +51     
- Misses      16530    16564      +34     
  Partials       98       98              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hSATAC hSATAC force-pushed the ash/eng-19927-fix-ai-json-not-guarenteed branch from 3d35c3e to a13b158 Compare April 28, 2026 14:09
@hSATAC hSATAC force-pushed the ash/eng-19927-only-retry-failed-maestro-flows-in-reuse_devices-mode branch from c0d875b to 0c794e5 Compare April 28, 2026 14:09
@hSATAC hSATAC force-pushed the ash/eng-19927-only-retry-failed-maestro-flows-in-reuse_devices-mode branch from 0c794e5 to 64b5fa4 Compare April 29, 2026 07:30
@hSATAC hSATAC force-pushed the ash/eng-19927-fix-ai-json-not-guarenteed branch 2 times, most recently from 10cf1db to d439776 Compare April 29, 2026 07:49
@hSATAC hSATAC requested a review from sjchmiela April 29, 2026 08:04
@hSATAC hSATAC marked this pull request as ready for review April 29, 2026 08:04
@hSATAC hSATAC force-pushed the ash/eng-19927-only-retry-failed-maestro-flows-in-reuse_devices-mode branch from 64b5fa4 to 08a9151 Compare April 30, 2026 07:27
@hSATAC hSATAC force-pushed the ash/eng-19927-fix-ai-json-not-guarenteed branch 2 times, most recently from 060eefd to 52a58b4 Compare April 30, 2026 07:44
@hSATAC hSATAC force-pushed the ash/eng-19927-only-retry-failed-maestro-flows-in-reuse_devices-mode branch from 08a9151 to 11f2a04 Compare April 30, 2026 07:44
@hSATAC hSATAC force-pushed the ash/eng-19927-fix-ai-json-not-guarenteed branch from 52a58b4 to e6705c9 Compare April 30, 2026 08:12
@hSATAC hSATAC force-pushed the ash/eng-19927-only-retry-failed-maestro-flows-in-reuse_devices-mode branch from aa0cf30 to e222a62 Compare April 30, 2026 09:38
@hSATAC hSATAC force-pushed the ash/eng-19927-fix-ai-json-not-guarenteed branch 2 times, most recently from 7448f87 to be9bd1a Compare April 30, 2026 09:48
@hSATAC hSATAC force-pushed the ash/eng-19927-only-retry-failed-maestro-flows-in-reuse_devices-mode branch from e222a62 to 14e6591 Compare April 30, 2026 09:48
Copy link
Copy Markdown
Contributor

@sjchmiela sjchmiela left a comment

Choose a reason for hiding this comment

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

a bit hard to wrap one's head around everything that's needed, but i trust you and your testing. i wish the junit output had everything we need. maybe we can submit a pull request to maestro?

Comment thread packages/build-tools/src/steps/functions/maestroFlowDiscovery.ts Outdated
import path from 'path';
import yaml from 'yaml';

const YAML_EXT = /\.ya?ml$/i;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: we can do manual === || === check which i think is slightly easier

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am leaning to keep this as this is simple enough and also handles uppercase/lowercase.

}
}

async function discoverFlows({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is the missing piece of information from maestro that could let us not have this? maybe they'd consider adding filepath to junit test results? i think this would fix the problem for us too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's actually a great idea.

Comment thread packages/build-tools/src/steps/functions/reportMaestroTestResults.ts Outdated
Base automatically changed from ash/eng-19927-only-retry-failed-maestro-flows-in-reuse_devices-mode to main May 5, 2026 09:27
@hSATAC hSATAC force-pushed the ash/eng-19927-fix-ai-json-not-guarenteed branch from be9bd1a to 7f3c9ce Compare May 5, 2026 09:28
Signed-off-by: Ash Wu <hsatac@gmail.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

⏩ The changelog entry check has been skipped since the "no changelog" label is present.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog PR that doesn't require a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants