Skip to content

Apply mocks to test command#42118

Merged
jonahwilliams merged 1 commit intoflutter:masterfrom
jonahwilliams:deflake_webfs_test
Oct 7, 2019
Merged

Apply mocks to test command#42118
jonahwilliams merged 1 commit intoflutter:masterfrom
jonahwilliams:deflake_webfs_test

Conversation

@jonahwilliams
Copy link
Copy Markdown
Contributor

Description

Missed the call to applyMocks. This causes several pieces of real functionality like pub to be run during the test, making it significantly more flaky than it needs to be.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 7, 2019
@jonahwilliams jonahwilliams requested a review from zanderso October 7, 2019 17:33
test('Builds a web bundle - end to end', () => testbed.run(() async {
final CommandRunner<void> runner = createTestCommandRunner(BuildCommand());
final BuildCommand buildCommand = BuildCommand();
applyMocksToCommand(buildCommand);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there any situation where we wouldn't want to do this? Is it possible to add an assert to createTestCommandRunner() to check that the mocks have been applied? If there are any, in the cases where mocks shouldn't be supplied, maybe we should have a call separate from createTestCommandRunner() that doesn't have the asserts.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Basically, "How do we make this less error prone?"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the best way would be to create a testbed specific command runner and enforce using that via import checks. For integration tests, I imagine we still want the capability to invoke commands directly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Filled #42151

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 7, 2019

Codecov Report

Merging #42118 into master will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #42118      +/-   ##
==========================================
+ Coverage   59.67%   59.72%   +0.04%     
==========================================
  Files         194      194              
  Lines       18977    18977              
==========================================
+ Hits        11325    11334       +9     
+ Misses       7652     7643       -9
Flag Coverage Δ
#flutter_tool 59.72% <ø> (+0.04%) ⬆️
Impacted Files Coverage Δ
...lutter_tools/lib/src/android/android_emulator.dart 35.41% <0%> (-31.25%) ⬇️
packages/flutter_tools/lib/src/web/web_device.dart 19.27% <0%> (-28.92%) ⬇️
packages/flutter_tools/lib/src/device.dart 58.82% <0%> (-2.95%) ⬇️
packages/flutter_tools/lib/src/version.dart 91.9% <0%> (-1.43%) ⬇️
packages/flutter_tools/lib/src/cache.dart 45.86% <0%> (-0.68%) ⬇️
packages/flutter_tools/lib/src/vmservice.dart 41.06% <0%> (-0.17%) ⬇️
packages/flutter_tools/lib/src/compile.dart 78.67% <0%> (+3.14%) ⬆️
packages/flutter_tools/lib/src/base/terminal.dart 80.76% <0%> (+3.84%) ⬆️
packages/flutter_tools/lib/src/base/io.dart 53.33% <0%> (+4.44%) ⬆️
packages/flutter_tools/lib/src/run_cold.dart 19.51% <0%> (+9.75%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fa4d31b...63b53f7. Read the comment docs.

@jonahwilliams jonahwilliams merged commit 9638756 into flutter:master Oct 7, 2019
@jonahwilliams jonahwilliams deleted the deflake_webfs_test branch October 7, 2019 22:20
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants