Skip to content

Conversation

@vashworth
Copy link
Contributor

@vashworth vashworth commented Aug 14, 2023

Fixes a couple of issues introduced in new iOS 17 physical device tooling: #131865.

  1. Duplicate messages were being filtered out too aggressively.

For example, if on the counter app, you printed "Increment!" on button click, it would only print once no matter how many times you clicked.

Sometimes more than one log source is used at a time and the original intention was to filter duplicates between two log sources, so it wouldn't print the same message from both logs. However, it would also filter when the same message was added more than once via the same log.

The new solution distinguishes a "primary" and a "fallback" log source and prefers to use the primary source unless it's not working, in which it'll use the fallback. If the fallback is faster than the primary, the primary will exclude the logs received by the fallback in a 1-to-1 fashion to prevent too-aggressive filtering. Once a flutter-message has been received by the primary source, fallback messages will be ignored.

Note: iOS < 17 did not regress.

  1. There was a race condition between the shutdown hooks and exiting XcodeDebug that was causing a crash when deleting a file that doesn't exist. This only affects CI - for the new integration tests and when testing with iOS 17 physical devices.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Aug 14, 2023
@vashworth vashworth changed the title fix duplicate messages and delete file race condition Fix log filtering and CI tests for iOS 17 physical devices Aug 14, 2023
@vashworth vashworth marked this pull request as ready for review August 14, 2023 16:52
expect(xcodeproj.existsSync(), isFalse);
expect(xcworkspace.existsSync(), isFalse);
expect(logger.errorText, isEmpty);
expect(fakeProcessManager, hasNoRemainingExpectations);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a nit (I know you probably just copied this), but the handling of process managers here is a little sus (here we're expecting that the fakeProcessManager that I think we never added any expectations, nor used, has no remaining expectations, while elsewhere we're constructing and injecting permissive FakeProcessManager.any() objects). I would recommend either (the easy solution) removing this expectation, or (the hard way) refactor this test to inject the fakeProcessManager we construct in the setup, and add FakeCommands if needed.

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@vashworth vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 14, 2023
@auto-submit auto-submit bot merged commit ec0b744 into flutter:master Aug 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2023
vashworth added a commit to vashworth/flutter that referenced this pull request Aug 14, 2023
…32491)

Fixes a couple of issues introduced in new iOS 17 physical device tooling: flutter#131865.

1) Duplicate messages were being filtered out too aggressively. 

For example, if on the counter app, you printed "Increment!" on button click, it would only print once no matter how many times you clicked.

Sometimes more than one log source is used at a time and the original intention was to filter duplicates between two log sources, so it wouldn't print the same message from both logs. However, it would also filter when the same message was added more than once via the same log.

The new solution distinguishes a "primary" and a "fallback" log source and prefers to use the primary source unless it's not working, in which it'll use the fallback. If the fallback is faster than the primary, the primary will exclude the logs received by the fallback in a 1-to-1 fashion to prevent too-aggressive filtering. Once a flutter-message has been received by the primary source, fallback messages will be ignored.

Note: iOS < 17 did not regress.

2) There was a race condition between the shutdown hooks and exiting XcodeDebug that was causing a crash when deleting a file that doesn't exist. This only affects CI  - for the new integration tests and when testing with iOS 17 physical devices.
vashworth added a commit to vashworth/flutter that referenced this pull request Aug 14, 2023
…32491)

Fixes a couple of issues introduced in new iOS 17 physical device tooling: flutter#131865.

1) Duplicate messages were being filtered out too aggressively. 

For example, if on the counter app, you printed "Increment!" on button click, it would only print once no matter how many times you clicked.

Sometimes more than one log source is used at a time and the original intention was to filter duplicates between two log sources, so it wouldn't print the same message from both logs. However, it would also filter when the same message was added more than once via the same log.

The new solution distinguishes a "primary" and a "fallback" log source and prefers to use the primary source unless it's not working, in which it'll use the fallback. If the fallback is faster than the primary, the primary will exclude the logs received by the fallback in a 1-to-1 fashion to prevent too-aggressive filtering. Once a flutter-message has been received by the primary source, fallback messages will be ignored.

Note: iOS < 17 did not regress.

2) There was a race condition between the shutdown hooks and exiting XcodeDebug that was causing a crash when deleting a file that doesn't exist. This only affects CI  - for the new integration tests and when testing with iOS 17 physical devices.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2023
vashworth added a commit to vashworth/flutter that referenced this pull request Aug 14, 2023
…32491)

Fixes a couple of issues introduced in new iOS 17 physical device tooling: flutter#131865.

1) Duplicate messages were being filtered out too aggressively. 

For example, if on the counter app, you printed "Increment!" on button click, it would only print once no matter how many times you clicked.

Sometimes more than one log source is used at a time and the original intention was to filter duplicates between two log sources, so it wouldn't print the same message from both logs. However, it would also filter when the same message was added more than once via the same log.

The new solution distinguishes a "primary" and a "fallback" log source and prefers to use the primary source unless it's not working, in which it'll use the fallback. If the fallback is faster than the primary, the primary will exclude the logs received by the fallback in a 1-to-1 fashion to prevent too-aggressive filtering. Once a flutter-message has been received by the primary source, fallback messages will be ignored.

Note: iOS < 17 did not regress.

2) There was a race condition between the shutdown hooks and exiting XcodeDebug that was causing a crash when deleting a file that doesn't exist. This only affects CI  - for the new integration tests and when testing with iOS 17 physical devices.
auto-submit bot pushed a commit that referenced this pull request Aug 14, 2023
Two CP commits in a single PR.

Original PRs: 
#131865
#132491
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 16, 2023
Manual roll Flutter from 9b6945b to f0e7c51 (63 revisions)

Manual roll requested by [email protected]

flutter/flutter@9b6945b...f0e7c51

2023-08-15 [email protected] Upgrade flutter packages. (flutter/flutter#132585)
2023-08-15 [email protected] [integration_test] Update outdated onScreenshot signature in README code snippet (flutter/flutter#132409)
2023-08-15 [email protected] Roll Flutter Engine from 0760b5589c71 to 22f03ffdc290 (1 revision) (flutter/flutter#132580)
2023-08-15 [email protected] Make completeError be a proper JS function (flutter/flutter#132492)
2023-08-15 [email protected] Analyze code snippets in flutter_driver docs (flutter/flutter#132337)
2023-08-15 [email protected] Skip backfiller for docs_publish (flutter/flutter#132510)
2023-08-15 [email protected] Further clarification of the TextSelectionControls migration (flutter/flutter#132539)
2023-08-15 [email protected] Roll Packages from 08080ab to 84218b9 (1 revision) (flutter/flutter#132572)
2023-08-15 [email protected] Roll Flutter Engine from 2c90670171a7 to 0760b5589c71 (1 revision) (flutter/flutter#132570)
2023-08-15 [email protected] Update `flutter_tools` internals related to Gradle/XCode to set `--local-engine-host`. (flutter/flutter#132346)
2023-08-15 [email protected] Roll Flutter Engine from 6bd7b717e110 to 2c90670171a7 (2 revisions) (flutter/flutter#132566)
2023-08-15 [email protected] Roll Flutter Engine from 9091107e594d to 6bd7b717e110 (1 revision) (flutter/flutter#132561)
2023-08-15 [email protected] Roll Flutter Engine from a17130ac4d49 to 9091107e594d (2 revisions) (flutter/flutter#132553)
2023-08-15 [email protected] Roll Flutter Engine from 6975ee1b6de7 to a17130ac4d49 (5 revisions) (flutter/flutter#132550)
2023-08-15 [email protected] Roll Flutter Engine from 21aed05ad751 to 6975ee1b6de7 (1 revision) (flutter/flutter#132543)
2023-08-15 [email protected] Manual roll of docker focal version. (flutter/flutter#132536)
2023-08-15 [email protected] Roll Flutter Engine from f1f912c0c7c7 to 21aed05ad751 (1 revision) (flutter/flutter#132540)
2023-08-15 [email protected] PaginatedDataTable improvements (flutter/flutter#131374)
2023-08-15 [email protected] Roll Flutter Engine from 1d63eece6c37 to f1f912c0c7c7 (2 revisions) (flutter/flutter#132538)
2023-08-15 [email protected] [devicelab] fix name of flutter driver benchmark. (flutter/flutter#132527)
2023-08-14 [email protected] Roll Flutter Engine from 3c8852827646 to 1d63eece6c37 (1 revision) (flutter/flutter#132534)
2023-08-14 [email protected] Updates `AutomatedTestWidgetsFlutterBinding.pump` to support microsecond precision (flutter/flutter#132401)
2023-08-14 [email protected] Roll Flutter Engine from 785b24900a36 to 3c8852827646 (2 revisions) (flutter/flutter#132530)
2023-08-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.21.3 to 2.21.4 (flutter/flutter#132525)
2023-08-14 [email protected] [devicelab] fix NAN/Inf in drawPoints benchmark. (flutter/flutter#132526)
2023-08-14 [email protected] Selection area should move selection word by word on a long press drag (flutter/flutter#132518)
2023-08-14 [email protected] Roll Flutter Engine from 56a0e2715dca to 785b24900a36 (10 revisions) (flutter/flutter#132522)
2023-08-14 [email protected] [flutter_tools] hide Skia specific rendering options. (flutter/flutter#132509)
2023-08-14 [email protected] Update menu examples for `SafeArea` (flutter/flutter#132390)
2023-08-14 [email protected] Update `dev/bots/test.dart` (and friends) to provide `--local-engine-host`. (flutter/flutter#132354)
2023-08-14 [email protected] Roll Flutter Engine from 46c11916f8d2 to 56a0e2715dca (2 revisions) (flutter/flutter#132508)
2023-08-14 [email protected] [devicelab] boot up benchmarks. (flutter/flutter#132148)
2023-08-14 [email protected] Roll Packages from 9b15c2e to 08080ab (5 revisions) (flutter/flutter#132505)
2023-08-14 [email protected] Roll Flutter Engine from c125e1e23d4b to 46c11916f8d2 (1 revision) (flutter/flutter#132502)
2023-08-14 [email protected] Fix log filtering and CI tests for iOS 17 physical devices (flutter/flutter#132491)
2023-08-14 [email protected] Roll Flutter Engine from 4e8b2b61f648 to c125e1e23d4b (1 revision) (flutter/flutter#132496)
2023-08-14 [email protected] Unpin leak_tracker and handle breaking changes in API. (flutter/flutter#132352)
2023-08-14 [email protected] Roll Flutter Engine from 62a392f29aa5 to 4e8b2b61f648 (2 revisions) (flutter/flutter#132490)
2023-08-14 [email protected] Roll Flutter Engine from f50ad637de75 to 62a392f29aa5 (1 revision) (flutter/flutter#132487)
2023-08-14 [email protected] Roll Flutter Engine from 9cebd2015688 to f50ad637de75 (3 revisions) (flutter/flutter#132480)
2023-08-14 [email protected] Roll Flutter Engine from bcaaa0e125fe to 9cebd2015688 (2 revisions) (flutter/flutter#132477)
2023-08-14 [email protected] Roll Flutter Engine from 7915c049a1fd to bcaaa0e125fe (1 revision) (flutter/flutter#132473)
2023-08-14 [email protected] Roll Flutter Engine from f47e864f2dcb to 7915c049a1fd (1 revision) (flutter/flutter#132466)
2023-08-13 [email protected] Revert "Fix `PopupMenuItem` & `CheckedPopupMenuItem` has redundant `ListTile` padding and update default horizontal padding for Material 3" (flutter/flutter#132457)
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
FlutterSu pushed a commit to flutter-rosita/flutter-rosita that referenced this pull request Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants