[CP-stable][ Device Lab ] Fix wakefulness check to only match log entries with string values #175357
Conversation
…tring values (flutter#174953) Sometimes there can be multiple wakefulness log entries (e.g., 'mWakefulness=Awake' and 'mWakefulness=1'), but we only care about the entry with a string value. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
@reidbaker 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. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the regular expression in _getWakefulness to more strictly match the device's wakefulness state from dumpsys power output. The new regex ensures that only lines with alphabetic values (like 'Awake' or 'Asleep') are matched, preventing potential issues with other numerical values. I have added one comment with a suggestion to further improve the robustness of the value parsing. Please note that the repository's style guide (line 25) indicates that cherry-picked changes are typically not reviewed.
| // A motoG4 phone returns `mWakefulness=Awake`. | ||
| // A Samsung phone returns `getWakefullnessLocked()=Awake`. | ||
| final RegExp wakefulnessRegexp = RegExp(r'.*(mWakefulness=|getWakefulnessLocked\(\)=).*'); | ||
| final RegExp wakefulnessRegexp = RegExp(r'(?:mWakefulness|getWakefulnessLocked\(\))=[a-zA-Z]+'); |
There was a problem hiding this comment.
The updated regex correctly narrows the match to alphabetic values. To make the parsing on the following line more robust, consider adding a capturing group for the value. This would allow extracting the value reliably with firstMatch and group(1) even if the line contains multiple = characters, making the code less dependent on the exact format of the dumpsys output.
| final RegExp wakefulnessRegexp = RegExp(r'(?:mWakefulness|getWakefulnessLocked\(\))=[a-zA-Z]+'); | |
| final RegExp wakefulnessRegexp = RegExp(r'(?:mWakefulness|getWakefulnessLocked\(\))=([a-zA-Z]+)'); |
reidbaker
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/175357, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
|
Adding emergency label because despite #175357 (comment) the git log shows c298091 as the previous commit which is from today. |
reidbaker
added
the
emergency
|
Detected the If you add the The "Merge" button is also unlocked. To bypass presubmits as well as the tree status, press the GitHub "Add to Merge Queue". |
reidbaker
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/175357, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
|
I'm force merging this as requested. |
…ch log entries with string values (flutter/flutter#175357)
5 participants
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?
#175356
Changelog Description:
Explain this cherry pick in one line that is accessible to most Flutter developers. See best practices for examples
no changelog needed ci only
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 production app (the app crashes on launch)
...
Workaround:
Is there a workaround for this issue?
none
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 post submits to see if pixel 7 pro tests pass