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

Conversation

@cbracken
Copy link
Member

@cbracken cbracken commented Dec 8, 2022

This is a minor cleanup that exposes the test metal surface to tests via EmbedderTestContextMeta::GetTestMetalSurface for parity with the GetTestMetalContext method which exposes the test metal context. This eliminates the need for the more specific GetTextureInfo method since the texture info is accessible via the test context.

This is a test refactoring with no semantic differences to the engine.

Issue: flutter/flutter#116381

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.

This is a minor cleanup that exposes the test metal surface to tests via
EmbedderTestContextMeta::GetTestMetalSurface for parity with the
GetTestMetalContext method which exposes the test metal context. This
eliminates the need for the more specific GetTextureInfo method since
the texture info is accessible via the test context.

This is a test refactoring with no semantic differences to the engine.

Issue: flutter/flutter#116381
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@flutter-dashboard flutter-dashboard bot added embedder Related to the embedder API needs tests labels Dec 8, 2022
@cbracken
Copy link
Member Author

cbracken commented Dec 8, 2022

This change touches only tests. I'll send a patch to flutter/cocoon to fix the bot so it ignores //flutter/shell/platform/embedder/{tests,fixtures}.

@cbracken cbracken merged commit 8a113d3 into flutter:main Dec 8, 2022
@cbracken cbracken deleted the test-metal-surface-cleanup branch December 8, 2022 18:23
@Hixie
Copy link
Contributor

Hixie commented Dec 8, 2022

test-exempt: is a test

@cbracken
Copy link
Member Author

cbracken commented Dec 8, 2022

For cross-reference, the patch to have the bot exempt this directory is: flutter/cocoon#2340

auto-submit bot pushed a commit to flutter/cocoon that referenced this pull request Dec 8, 2022
Embedder API tests in the engine are implemented as C++ unit tests and
test infrastructure such as embedder_test_context_metal.{h,cc} (located
under the shell/platform/embedder/tests directory), which execute and
test Dart functions (and associated test assets) stored in the
shell/platform/embedder/fixtures directory.

Uncovered while sending flutter/engine#38133.

Issue: flutter/flutter#116381
cbracken added a commit that referenced this pull request Dec 8, 2022
While I've sent a patch to mark the shell/platform/embedder/tests and
fixtures directories as test exempt (since they are tests), by
convention, tests should end in _unittests.* for C++ tests, and _test.*
for Dart tests. This renames for consistency with other tests such as
embedder_a11y_unittests.cc.

Uncovered by #38133

Related: flutter/cocoon#2340
Issue: flutter/flutter#116381
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 9, 2022
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Dec 9, 2022
* bd8bcf9 Roll Fuchsia Mac SDK from crEcyXdyZ686cAqMV... to pMV6A0ykZQ8aA3NG2... (flutter/engine#38120)

* dec8b52 Preliminary implementation of UIA for A11y on Windows (flutter/engine#37754)

* 5545ccf Roll Fuchsia Linux SDK from NlJGkMbtZqQ6_BCpu... to xn8ztWtp-zww-jObz... (flutter/engine#38122)

* 80a15a4 Create FlutterActivity/FlutterFragment using light weight engine with FlutterEngineGroup (flutter/engine#36963)

* 5caef85 Full implementation of text-input-test (flutter/engine#37986)

* 8f6036e Reland fix wrong VSYNC event (flutter/engine#37865)

* 4101c36 [iOS] Change locale format for spell check (flutter/engine#38080)

* 2f5b67e [embedder] Ensure destruction called on present (flutter/engine#38078)

* 0bddc60 [Impeller Scene] Depth attachment; baked lighting example (flutter/engine#38118)

* 6aa4ccd Remove dlCanvasRecorder from flutter::PictureRecorder (flutter/engine#38127)

* dbb5284 [Windows] Add more cursor plugin tests (flutter/engine#38112)

* 6e91204 Roll Fuchsia Mac SDK from pMV6A0ykZQ8aA3NG2... to 9SnrQ0vbR8IC7UIoP... (flutter/engine#38135)

* 3140ad9 [Impeller] order metal samplers according to declared order and not usage order (flutter/engine#38115)

* 84abf21 Remove autoninja. (flutter/engine#38136)

* 8a113d3 [embedder] Expose metal surface from test context (flutter/engine#38133)

* 1ef25b6 Roll Fuchsia Mac SDK from 9SnrQ0vbR8IC7UIoP... to aMW0DjntzFJj4RoR3... (flutter/engine#38139)

* 748b3bc Revert "Remove dlCanvasRecorder from flutter::PictureRecorder (#38127)" (flutter/engine#38137)

* b6daf3d [embedder] Consistent naming for GL/Metal tests (flutter/engine#38141)

* 339d04b [web] Trivial fix for non-static interop JS interop class. (flutter/engine#38126)

* 1fcbb9c [tools] Eliminate version on Obj-C docs (flutter/engine#38145)

* 71928b6 [Impeller] Use DrawPath instead of Rect geometry when the paint style is stroke (flutter/engine#38146)

* 23ce8fd Roll Skia from dd3285a80b23 to f84dc9303045 (4 revisions) (flutter/engine#38123)

* 366f866 Roll Skia from f84dc9303045 to 2691cd7b4110 (40 revisions) (flutter/engine#38151)

* 447e701 Roll Skia from 2691cd7b4110 to 711396b81248 (1 revision) (flutter/engine#38152)

* cd5d91b Pylint testing/run_tests.py (flutter/engine#38016)

* aafac08 Roll Skia from 711396b81248 to b253b10374e7 (7 revisions) (flutter/engine#38157)

* 799dc78 Roll Fuchsia Linux SDK from xn8ztWtp-zww-jObz... to rRJIjuO-dPNCpCTd9... (flutter/engine#38134)

* 3aa3d2a Massage the JS interop around `didCreateEngineInitializer` (flutter/engine#38147)

* 030950f Roll Skia from b253b10374e7 to ec407902999b (3 revisions) (flutter/engine#38158)
gspencergoog pushed a commit to gspencergoog/flutter that referenced this pull request Jan 19, 2023
…16802)

* bd8bcf9 Roll Fuchsia Mac SDK from crEcyXdyZ686cAqMV... to pMV6A0ykZQ8aA3NG2... (flutter/engine#38120)

* dec8b52 Preliminary implementation of UIA for A11y on Windows (flutter/engine#37754)

* 5545ccf Roll Fuchsia Linux SDK from NlJGkMbtZqQ6_BCpu... to xn8ztWtp-zww-jObz... (flutter/engine#38122)

* 80a15a4 Create FlutterActivity/FlutterFragment using light weight engine with FlutterEngineGroup (flutter/engine#36963)

* 5caef85 Full implementation of text-input-test (flutter/engine#37986)

* 8f6036e Reland fix wrong VSYNC event (flutter/engine#37865)

* 4101c36 [iOS] Change locale format for spell check (flutter/engine#38080)

* 2f5b67e [embedder] Ensure destruction called on present (flutter/engine#38078)

* 0bddc60 [Impeller Scene] Depth attachment; baked lighting example (flutter/engine#38118)

* 6aa4ccd Remove dlCanvasRecorder from flutter::PictureRecorder (flutter/engine#38127)

* dbb5284 [Windows] Add more cursor plugin tests (flutter/engine#38112)

* 6e91204 Roll Fuchsia Mac SDK from pMV6A0ykZQ8aA3NG2... to 9SnrQ0vbR8IC7UIoP... (flutter/engine#38135)

* 3140ad9 [Impeller] order metal samplers according to declared order and not usage order (flutter/engine#38115)

* 84abf21 Remove autoninja. (flutter/engine#38136)

* 8a113d3 [embedder] Expose metal surface from test context (flutter/engine#38133)

* 1ef25b6 Roll Fuchsia Mac SDK from 9SnrQ0vbR8IC7UIoP... to aMW0DjntzFJj4RoR3... (flutter/engine#38139)

* 748b3bc Revert "Remove dlCanvasRecorder from flutter::PictureRecorder (flutter#38127)" (flutter/engine#38137)

* b6daf3d [embedder] Consistent naming for GL/Metal tests (flutter/engine#38141)

* 339d04b [web] Trivial fix for non-static interop JS interop class. (flutter/engine#38126)

* 1fcbb9c [tools] Eliminate version on Obj-C docs (flutter/engine#38145)

* 71928b6 [Impeller] Use DrawPath instead of Rect geometry when the paint style is stroke (flutter/engine#38146)

* 23ce8fd Roll Skia from dd3285a80b23 to f84dc9303045 (4 revisions) (flutter/engine#38123)

* 366f866 Roll Skia from f84dc9303045 to 2691cd7b4110 (40 revisions) (flutter/engine#38151)

* 447e701 Roll Skia from 2691cd7b4110 to 711396b81248 (1 revision) (flutter/engine#38152)

* cd5d91b Pylint testing/run_tests.py (flutter/engine#38016)

* aafac08 Roll Skia from 711396b81248 to b253b10374e7 (7 revisions) (flutter/engine#38157)

* 799dc78 Roll Fuchsia Linux SDK from xn8ztWtp-zww-jObz... to rRJIjuO-dPNCpCTd9... (flutter/engine#38134)

* 3aa3d2a Massage the JS interop around `didCreateEngineInitializer` (flutter/engine#38147)

* 030950f Roll Skia from b253b10374e7 to ec407902999b (3 revisions) (flutter/engine#38158)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

embedder Related to the embedder API needs tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants