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

Conversation

dkwingsmt
Copy link
Contributor

@dkwingsmt dkwingsmt commented Aug 30, 2023

The issue that blocks these mocking methods have been resolved.

However, there is one more mention of this issue in run_tests.py and I have no idea why it was marked so. It was originally added here.

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 Hixie said 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.

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

There are tons of these everywhere in the engine still. Should we just fix all of those too?

@dkwingsmt
Copy link
Contributor Author

Sure! Let me do a full search.

MOCK_METHOD3(CopyHostBuffer,
bool(const uint8_t* source, Range source_range, size_t offset));
// MOCK_METHOD(bool, CopyHostBuffer, (const uint8_t* source, Range
// source_range, size_t offset), (override));
Copy link
Contributor Author

@dkwingsmt dkwingsmt Sep 1, 2023

Choose a reason for hiding this comment

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

Since this method is not virtual, and its mock is not used, this mock is removed.

MOCK_METHOD(void, SetLabel, (std::string_view label), (override));
// MOCK_METHOD(bool, SetContents, (const uint8_t* contents, size_t length,
// size_t slice), (override)); MOCK_METHOD(bool, SetContents,
// (std::shared_ptr<const fml::Mapping> mapping, size_t slice), (override));
Copy link
Contributor Author

@dkwingsmt dkwingsmt Sep 1, 2023

Choose a reason for hiding this comment

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

Since this method is not virtual, and its mock is not used, this mock is removed.

@dkwingsmt
Copy link
Contributor Author

dkwingsmt commented Sep 1, 2023

@chinmaygarde I've converted all current cases using regex. There are a few cases that might need manual review.


// Return the currently configured PlatformWindow.
PlatformWindow GetPlatformWindow() const;
virtual PlatformWindow GetPlatformWindow() const;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also this method is now marked as virtual, since its mock is used.

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

Looks like the presubs are mad at a couple of Windows unittests. They seem straightforward though.

Thanks so much for doing this!

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 2, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Sep 2, 2023
…sions) (#133924)

Manual roll requested by [email protected]

flutter/engine@489c399...e496eec

2023-09-02 [email protected] Roll Skia from 2d8849f9f0cc to 15f77147a3ec (1 revision) (flutter/engine#45414)
2023-09-02 [email protected] Roll Fuchsia Mac SDK from OF4TS05qlWCjukWw6... to MesZPNdj-uw8VdCyV... (flutter/engine#45413)
2023-09-02 [email protected] Remove --disable-service-auth-codes (flutter/engine#45356)
2023-09-02 [email protected] [Impeller] Import cstring for memcpy. (flutter/engine#45408)
2023-09-02 [email protected] Roll Dart SDK from cdf1ce0c6d7e to a5c7102af509 (1 revision) (flutter/engine#45412)
2023-09-02 [email protected] Roll ANGLE from 179bd7762ffa to ebf1e7163216 (1 revision) (flutter/engine#45411)
2023-09-02 [email protected] Remove deprecated MOCK_METHODx calls (flutter/engine#45307)
2023-09-02 [email protected] [Impeller] Better demonstrate blur and draw picture? (flutter/engine#45388)
2023-09-02 [email protected] [Impeller] Make paths externally immutable, update all tests to use PathBuilder to create Path. (flutter/engine#45393)
2023-09-02 [email protected] Roll ANGLE from 962fdf7b7882 to 179bd7762ffa (1 revision) (flutter/engine#45409)
2023-09-02 [email protected] Cull the RTree bounds when they are forwarded in DrawDisplayList (flutter/engine#45358)
2023-09-02 [email protected] Roll Skia from fedff79a6afc to 2d8849f9f0cc (3 revisions) (flutter/engine#45407)
2023-09-02 [email protected] [impeller] premultiply vertices colors. (flutter/engine#45406)
2023-09-01 [email protected] Roll ANGLE from 6a09e41ce6ea to 962fdf7b7882 (224 revisions) (flutter/engine#45400)
2023-09-01 [email protected] Roll Skia from 22ae23891e8e to fedff79a6afc (1 revision) (flutter/engine#45405)
2023-09-01 [email protected] [Impeller] turned on validations for all debug builds (flutter/engine#45350)
2023-09-01 [email protected] Roll Fuchsia Mac SDK from sk7JBGzW1Jw10Wy-T... to OF4TS05qlWCjukWw6... (flutter/engine#45403)
2023-09-01 [email protected] Roll Skia from 2c0405489966 to 22ae23891e8e (1 revision) (flutter/engine#45402)
2023-09-01 [email protected] [Windows] Update vsync on raster thread (flutter/engine#45310)
2023-09-01 [email protected] Roll Dart SDK from a2ea759c16cc to cdf1ce0c6d7e (1 revision) (flutter/engine#45397)
2023-09-01 [email protected] Roll Skia from f3f6c733c7e6 to 2c0405489966 (1 revision) (flutter/engine#45396)
2023-09-01 [email protected] Roll Skia from 02fa14799c6c to f3f6c733c7e6 (1 revision) (flutter/engine#45394)
2023-09-01 [email protected] Roll Skia from d5d3b0d4ee77 to 02fa14799c6c (2 revisions) (flutter/engine#45392)
2023-09-01 [email protected] [ios][ios17][text_input]fix text input system highlight in iOS 17 Beta 7 with firstRectForRange (flutter/engine#45303)
2023-09-01 [email protected] Roll Skia from d6266ef14a7e to d5d3b0d4ee77 (2 revisions) (flutter/engine#45389)
2023-09-01 [email protected] Roll Dart SDK from 0c121a6431cc to a2ea759c16cc (1 revision) (flutter/engine#45384)

Also rolling transitive DEPS:
  fuchsia/sdk/core/mac-amd64 from sk7JBGzW1Jw1 to MesZPNdj-uw8

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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
dkwingsmt added a commit that referenced this pull request Sep 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants