Repository navigation
[CP-stable][tool] Accept plutil diagnostics from stderr in plist_parser_test (macOS 26) - #195342
Conversation
…cOS 26) (flutter#193292) Fixes flutter#193159 ## What's failing Two tests in `plist_parser_test.dart` fail on macOS 26 (first seen on `staging.shadow/Mac tool_integration_tests_1` at Mac-26.6.2 / Xcode 17C52, https://ci.chromium.org/b/8670252478857878881): - `PlistParser.getValueFromFile<String> returns null for a malformed plist file` - `PlistParser.replaceKey returns false for a malformed plist file` Both tests still get the right result (`null` / `false`, and `plutil` still exits 1). What changed is where `plutil` prints its error: - Before macOS 26, `plutil` printed `<path>: Property List error: … / JSON error: …` on **stdout**. - On macOS 26, `plutil` was rewritten (Apple open-sourced the new version in swift-corelibs-foundation, `Sources/plutil/PLUContext.swift`) and the same message now goes to **stderr**. `ProcessUtils.runSync` logs a failed command's stdout with `printStatus` and its stderr with `printError`. So on macOS 26 `logger.statusText` is empty and the diagnostic shows up in `logger.errorText` instead, ahead of the `ProcessException:` line. The tests only looked at `statusText`. flutter#172913 ran into the same plutil change last year in `xcode_backend.dart`. ## The fix Test-only change. `PlistParser` itself is fine. - Look for the plutil diagnostic in `statusText + errorText` rather than `statusText` alone, so the test passes whichever stream plutil uses. The full message is still asserted, so a change in the wording would still be caught. - Check that `errorText` ends with the `ProcessException` block instead of equalling it, since on macOS 26 the plutil message now comes first. ## How I verified it - Ran the tests locally. - led run of `Mac tool_integration_tests_1` (subshard `1_10`, the one that contains `plist_parser_test.dart`) on a Mac-26.6.2 bot (`mac-340-g140`, the same bot as the original failure) at this PR's head: https://ci.chromium.org/b/8669753954317678657 — green, both tests pass. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
|
@vashworth please fill out the PR description above, afterwards the release team will review this request. |
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request updates plist_parser_test.dart to handle diagnostic output changes introduced in macOS 26, where plutil writes diagnostics to stderr instead of stdout. The tests now combine logger.statusText and logger.errorText for diagnostic assertions and use endsWith instead of exact equality for error text assertions. There are no review comments, and I have no feedback to provide.
ca7e14a
into
flutter:flutter-3.47-candidate.0
This pull request is created by automatic cherry pick workflow
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.
Issue Link:
What is the link to the issue this cherry-pick is addressing?
#193159
Impact Description:
What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)?
Does it impact development (ex. flutter doctor crashes when Android Studio is installed),
or the shipping of production apps (the app crashes on launch).
This information is for domain experts and release engineers to understand the consequences of saying yes or no to the cherry pick.
CI test-only fix to refactor
plist_parser_test.dartto pass on macOS 26.Changelog Description:
Explain this cherry pick:
See best practices for examples.
NA - CI test-only fix
Workaround:
Is there a workaround for this issue?
No
Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
What are the steps to validate that this fix works?
Run
plist_parser_test.darttests in CI or locally on macOS 26