Skip to content
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

Always fake ProcessManager when you fake Filesystem in tests #42369

Merged
merged 1 commit into from Oct 11, 2019

Conversation

Hixie
Copy link
Contributor

@Hixie Hixie commented Oct 9, 2019

...because otherwise, processes that think they're manipulating your
filesystem will be doing crazy things the test is ignoring, leading to
(at best) failures and (at worst) flakes or disk corruption.

@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Oct 9, 2019
@Hixie
Copy link
Contributor Author

Hixie commented Oct 9, 2019

cc @jonahwilliams this fixes the ThrowingPub layering issue too

@codecov
Copy link

codecov bot commented Oct 9, 2019

Codecov Report

Merging #42369 into master will decrease coverage by 1.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #42369      +/-   ##
==========================================
- Coverage   60.48%   59.38%   -1.11%     
==========================================
  Files         194      194              
  Lines       18856    18856              
==========================================
- Hits        11405    11197     -208     
- Misses       7451     7659     +208
Flag Coverage Δ
#flutter_tool 59.38% <100%> (-1.11%) ⬇️
Impacted Files Coverage Δ
packages/flutter_tools/lib/src/run_hot.dart 69.48% <100%> (+0.06%) ⬆️
packages/flutter_tools/lib/src/ios/simulators.dart 6.9% <0%> (-43.82%) ⬇️
...ackages/flutter_tools/lib/src/resident_runner.dart 42.82% <0%> (-14.55%) ⬇️
packages/flutter_tools/lib/src/base/os.dart 24.6% <0%> (-5.56%) ⬇️
packages/flutter_tools/lib/src/base/io.dart 59.45% <0%> (-5.41%) ⬇️
...ools/lib/src/build_runner/resident_web_runner.dart 76.16% <0%> (-2.91%) ⬇️
packages/flutter_tools/lib/src/version.dart 93.23% <0%> (-1.45%) ⬇️
...ges/flutter_tools/lib/src/application_package.dart 65.94% <0%> (-1.09%) ⬇️
packages/flutter_tools/lib/src/cache.dart 48.47% <0%> (-0.71%) ⬇️
packages/flutter_tools/lib/src/base/utils.dart 95.47% <0%> (-0.51%) ⬇️
... and 3 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 53be552...fce8013. Read the comment docs.

// Copyright 2016 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

Copy link
Member

Choose a reason for hiding this comment

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

This file needs comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

Cool, thanks. This is a really nice API, but it doesn't appear to have any non-trivial uses in this PR. What are your plans for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My first customer is #42491. I suspect the API will evolve as it gets more users.

@Hixie Hixie force-pushed the pubspec.timestamp branch 2 times, most recently from 68abb15 to 9e5d2e2 Compare October 11, 2019 01:58
...because otherwise, processes that think they're manipulating your
filesystem will be doing crazy things the test is ignoring, leading to
(at best) failures and (at worst) flakes or disk corruption.
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm

@Hixie Hixie merged commit 08643c4 into flutter:master Oct 11, 2019
@Hixie Hixie deleted the pubspec.timestamp branch October 11, 2019 18:23
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
…#42369)

...because otherwise, processes that think they're manipulating your
filesystem will be doing crazy things the test is ignoring, leading to
(at best) failures and (at worst) flakes or disk corruption.
@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.

None yet

4 participants