Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[macOS] Fix tests failing on Sonoma #46461

Merged
merged 1 commit into from
Oct 18, 2023
Merged

Conversation

knopp
Copy link
Member

@knopp knopp commented Oct 2, 2023

  1. Using arbitrary struct passed as const reference to OCMStub now fails in OCMock. Down the line this will result with object_getClass being called in OCMArg.m with the address of the reference, which is not a valid class instance. This seems to have worked pre-sonoma, but it seems like a weird thing to rely on.

  2. NSResponder mock can not be set to view controller anymore. The controller will try to access an ivar of the NSResponder, but mocked responder does not have the ivar of original objects which will result on invalid selector being called on a NSMutableArray one of the ivar of mock objects. Solution for this is to inherit from NSResponder and forward calls to mocked object.

  3. Adding flutter::kModifierFlagShiftLeft to a modifier flag containing kCGEventFlagMaskShift. The assertion was introduced in [macOS] Synchronise modifiers from mouse events for RawKeyboard #46230 but i missed the test failure because of the problems above.

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

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

@knopp
Copy link
Member Author

knopp commented Oct 2, 2023

Btw, Chromium seems to have patched the OCMArg problem: https://chromium-review.googlesource.com/c/chromium/src/+/3011651

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

Thanks for the additional fix for the missing shift modifier.

@cbracken cbracken added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 18, 2023
@auto-submit auto-submit bot merged commit 5cacdd4 into flutter:main Oct 18, 2023
@cbracken
Copy link
Member

Thanks again for the fix; this is additional justification for us to find time to dig into the non-hermetic test situation in the macOS embedder tests so we can re-enable the presubmits.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 18, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 18, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 18, 2023
…136789)

flutter/engine@1de09d1...0c1c292

2023-10-18 [email protected] Roll Skia from 8796ee7d1c66 to 3d938d4b00ee (3 revisions) (flutter/engine#47042)
2023-10-18 [email protected] Roll Skia from 13694b8c64aa to 8796ee7d1c66 (1 revision) (flutter/engine#47041)
2023-10-18 [email protected] [macOS] Fix tests failing on Sonoma (flutter/engine#46461)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
harryterkelsen pushed a commit that referenced this pull request Oct 23, 2023
1. Using arbitrary struct passed as const reference  to `OCMStub` now fails in OCMock. Down the line this will result with `object_getClass` being called in [`OCMArg.m`](https://github.com/erikdoe/ocmock/blob/master/Source/OCMock/OCMArg.m#L129-L133) with the address of the reference, which is not a valid class instance. This seems to have worked pre-sonoma, but it seems like a weird thing to rely on.

2. `NSResponder` mock can not be set to view controller anymore. The controller will try to access an ivar of the `NSResponder`, but mocked responder does not have the ivar of original objects which will result on invalid selector being called on a `NSMutableArray` one of the ivar of mock objects. Solution for this is to inherit from `NSResponder` and forward calls to mocked object.

3. Adding `flutter::kModifierFlagShiftLeft` to a modifier flag containing `kCGEventFlagMaskShift`. The assertion was introduced in #46230 but i missed the test failure because of the problems above.

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects: desktop autosubmit Merge PR when tree becomes green via auto submit App platform-macos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants