Skip to content

/ engine Public

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Factor out a task synchronization help function for unit tests #28467

Merged
merged 3 commits into from Sep 8, 2021
Merged

Factor out a task synchronization help function for unit tests #28467

merged 3 commits into from Sep 8, 2021
+62 −73

Conversation

@ColdPaleLight
Copy link
Member

@ColdPaleLight ColdPaleLight commented Sep 5, 2021 •
edited

The pattern of synchronously executing a task happens multiple times in tests. Pulled out helper functions can make tests are easier to parse/maintain

fixes: flutter/flutter#89493

detail: #28159 (comment)

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.
@google-cla google-cla bot added the cla: yes label Sep 5, 2021
@ColdPaleLight
ColdPaleLight reviewed Sep 5, 2021
View changes
testing/BUILD.gn
"post_task_sync.cc",
"post_task_sync.h",
Comment on lines +20 to +21

This comment has been minimized.

Sign in to view
@ColdPaleLight

ColdPaleLight Sep 5, 2021
Author Member

I am not sure if this is a good place to add post_task_sync.h and post_task_sync.cc. In addition, I am not sure whether post_task_sync.h and post_task_sync.cc are good file names.
But I think we should put these helper functions in a public place like this so that other unit tests can use them directly

This comment has been minimized.

Sign in to view
@gaaclarke

gaaclarke Sep 8, 2021
Contributor

Yea, this sounds good to me.
@ColdPaleLight ColdPaleLight requested a review from gaaclarke Sep 5, 2021
@ColdPaleLight
Factor out several task synchronization help functions for unit tests
6f712c9
@ColdPaleLight ColdPaleLight force-pushed the ColdPaleLight:post_task_sync branch from 53fb2ba to 6f712c9 Sep 5, 2021
@ColdPaleLight ColdPaleLight mentioned this pull request Sep 5, 2021
Merged
7 of 8 tasks
@gaaclarke
gaaclarke requested changes Sep 7, 2021
View changes
testing/post_task_sync.h Outdated
void PostPlatformTaskSync(const TaskRunners& task_runners,
const std::function<void()>& function);

void PostUITaskSync(const TaskRunners& task_runners,
const std::function<void()>& function);

void PostRasterTaskSync(const TaskRunners& task_runners,
const std::function<void()>& function);

void PostIOTaskSync(const TaskRunners& task_runners,
const std::function<void()>& function);
Comment on lines 14 to 24

This comment has been minimized.

Sign in to view
@gaaclarke

gaaclarke Sep 7, 2021
Contributor

I think we should remove these functions and just rely on PostTaskSync

This comment has been minimized.

Sign in to view
@ColdPaleLight

ColdPaleLight Sep 8, 2021
Author Member

Done
@ColdPaleLight
Remove redundant functions
d21eb8e
@ColdPaleLight ColdPaleLight changed the title Factor out several task synchronization help functions for unit tests Factor out a task synchronization help function for unit tests Sep 8, 2021
@ColdPaleLight ColdPaleLight requested a review from gaaclarke Sep 8, 2021
@ColdPaleLight
Delete unused header file
66f6a6a
@gaaclarke
gaaclarke approved these changes Sep 8, 2021
View changes
Copy link
Contributor

@gaaclarke gaaclarke left a comment

LGTM, nice
@fluttergithubbot fluttergithubbot merged commit e296708 into flutter:master Sep 8, 2021
21 checks passed
21 checks passed
@flutter-dashboard
Linux Android AOT Engine
Details
@flutter-dashboard
Linux Android Debug Engine
Details
@flutter-dashboard
Linux Arm Host Engine
Details
@flutter-dashboard
Linux Framework Smoke Tests
Details
@flutter-dashboard
Linux Fuchsia
Details
@flutter-dashboard
Linux Fuchsia FEMU
Details
@flutter-dashboard
Linux Host Engine
Details
@flutter-dashboard
Linux Unopt
Details
@flutter-dashboard
Mac Android AOT Engine
Details
@flutter-dashboard
Mac Host Engine
Details
@flutter-dashboard
Mac Unopt
Details
@flutter-dashboard
Mac iOS Engine
Details
@wip
WIP Ready for review
Details
@flutter-dashboard
Windows Android AOT Engine
Details
@flutter-dashboard
Windows Host Engine
Details
@flutter-dashboard
Windows UWP Engine
Details
@flutter-dashboard
Windows Unopt
Details
@cirrus-ci
build_and_test_linux_unopt_debug Task Summary
Details
@flutter-dashboard
ci.yaml validation .ci.yaml validation
Details
@google-cla
cla/google All necessary CLAs are signed
@flutter-dashboard
luci-engine
Details
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
2d3dcae 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
b0848c7 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
e484a61 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
e5fbe01 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
d122062 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
735398c 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
6d05ddb 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
e0771ce 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
61c961e 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
fb62c99 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
8d56c75 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
70f748b 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
65d5e0d 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
1a19572 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
bf6d98f 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
1c30a09 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
12fd759 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
75408ca 
…sts (flutter/engine#28467)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 10, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 10, 2021
@engine-flutter-autoroll
e296708d0 Factor out a task synchronization help function for unit te…
9c3d8ce 
…sts (flutter/engine#28467)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gaaclarke gaaclarke

Assignees
No one assigned
Labels
cla: yes waiting for tree to go green
Projects
None yet
Milestone
No milestone
3 participants
@ColdPaleLight @gaaclarke @fluttergithubbot