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

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

Conversation

@ColdPaleLight
Copy link
Member

@ColdPaleLight ColdPaleLight commented Sep 5, 2021 •
edited
Loading

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 commented Sep 5, 2021
View reviewed changes
testing/BUILD.gn
Comment on lines +20 to +21
"post_task_sync.cc",
"post_task_sync.h",
Copy link
Member Author

@ColdPaleLight ColdPaleLight Sep 5, 2021

Choose a reason for hiding this comment

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

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

Copy link
Member

@gaaclarke gaaclarke Sep 8, 2021

Choose a reason for hiding this comment

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

Yea, this sounds good to me.

@ColdPaleLight ColdPaleLight mentioned this pull request Sep 5, 2021
Merged
8 tasks
gaaclarke
gaaclarke suggested changes Sep 7, 2021
View reviewed changes
testing/post_task_sync.h Outdated
Comment on lines 14 to 24
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);
Copy link
Member

@gaaclarke gaaclarke Sep 7, 2021

Choose a reason for hiding this comment

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

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

Copy link
Member Author

@ColdPaleLight ColdPaleLight Sep 8, 2021

Choose a reason for hiding this comment

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

Done

@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
gaaclarke
gaaclarke approved these changes Sep 8, 2021
View reviewed changes
Copy link
Member

@gaaclarke gaaclarke left a comment

Choose a reason for hiding this comment

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

LGTM, nice

@gaaclarke gaaclarke added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Sep 8, 2021
@fluttergithubbot fluttergithubbot merged commit e296708 into flutter:master Sep 8, 2021
@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
e296708 Factor out a task synchronization help function for unit tests (
2d3dcae 
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
e296708 Factor out a task synchronization help function for unit tests (
b0848c7 
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
e296708 Factor out a task synchronization help function for unit tests (
e484a61 
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
e296708 Factor out a task synchronization help function for unit tests (
e5fbe01 
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
e296708 Factor out a task synchronization help function for unit tests (
d122062 
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
e296708 Factor out a task synchronization help function for unit tests (
735398c 
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
e296708 Factor out a task synchronization help function for unit tests (
6d05ddb 
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
e296708 Factor out a task synchronization help function for unit tests (
e0771ce 
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
e296708 Factor out a task synchronization help function for unit tests (
61c961e 
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
e296708 Factor out a task synchronization help function for unit tests (
fb62c99 
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
e296708 Factor out a task synchronization help function for unit tests (
8d56c75 
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
e296708 Factor out a task synchronization help function for unit tests (
70f748b 
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
e296708 Factor out a task synchronization help function for unit tests (
65d5e0d 
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
e296708 Factor out a task synchronization help function for unit tests (
1a19572 
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
e296708 Factor out a task synchronization help function for unit tests (
bf6d98f 
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
e296708 Factor out a task synchronization help function for unit tests (
1c30a09 
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
e296708 Factor out a task synchronization help function for unit tests (
12fd759 
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
e296708 Factor out a task synchronization help function for unit tests (
75408ca 
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
e296708 Factor out a task synchronization help function for unit tests (
9c3d8ce 
flutter/engine#28467)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@gaaclarke gaaclarke gaaclarke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes customer: alibaba waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consider factoring out several task synchronization help functions for unit tests

3 participants

@ColdPaleLight @gaaclarke @fluttergithubbot