Catch error for missing directory in `FontConfigManager` #138496

eliasyishak · 2023-11-15T17:46:57Z

Closes:

[flutter_tools] PathNotFoundException during FontConfigManager.dispose #138434

We will catch any errors while attempting to clear the temp directories that don't exist for the FontConfigManager class

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, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

packages/flutter_tools/lib/src/test/font_config_manager.dart

christopherfujino · 2023-11-15T18:28:28Z

packages/flutter_tools/test/general.shard/flutter_tester_device_test.dart

+    final Directory fontsDirectory = fileSystem.file(fontConfigManager.fontConfigFile).parent;
+    fontsDirectory.deleteSync(recursive: true);
+
+    expect(fontConfigManager.dispose, returnsNormally);


returnsNormally is not an async matcher, so this is testing that dispose returns a future without throwing, not that the future does not complete with an error. why don't you just call the function and await the result without wrapping it in an expect (if it throws, the test will still fail).

Ah that is a good point on the returnsNormally.. but if I did the below

testUsingContext('Missing dir error caught for FontConfigManger.dispose', () async { final FontConfigManager fontConfigManager = FontConfigManager(); final Directory fontsDirectory = fileSystem.file(fontConfigManager.fontConfigFile).parent; fontsDirectory.deleteSync(recursive: true); await fontConfigManager.dispose(); }, overrides: <Type, Generator>{ FileSystem: () => fileSystem, ProcessManager: () => processManager, });

And if we happen to throw for an exception (other than FileSystemException), wouldn't it crash for that file? ie. it won't run the other tests below the one above

exceptions won't crash the test runner, but will be treated as ~~passes~~ failures.

Hmm maybe I'm messing up something but I'm trying to get the rest of the tests to run after purposefully throwing an Exception within the dispose method, I have edited it to have the below

Future<void> dispose() async { if (_fontsDirectory != null) { globals.printTrace('Deleting ${_fontsDirectory!.path}...'); try { await _fontsDirectory!.delete(recursive: true); } on FileSystemException { // Silently exit } _fontsDirectory = null; throw Exception('error'); // Purposefully thrown } }

When I try to run the test file with flutter test test/general.shard/flutter_tester_device_test.dart, the process hangs after the first uncaught exception.

Same result for dart test test/general.shard/flutter_tester_device_test.dart

However, with the expect(fontConfigManager.dispose, returnsNormally);, it does highlight the error and it continues with the rest of the tests in file. Am I missing something? The logs below show the output from the test runner when using the returnsNormally using flutter test

eliasyishak-macbookpro:flutter_tools eliasyishak$ flutter test /Users/eliasyishak/Desktop/flutter/packages/flutter_tools/test/general.shard/flutter_tester_device_test.dart 00:00 +0: Missing dir error caught for FontConfigManger.dispose Invalid argument(s): error #0 FontConfigManager.dispose (package:flutter_tools/src/test/font_config_manager.dart:43:7) <asynchronous suspension> 00:01 +0 -1: Missing dir error caught for FontConfigManger.dispose [E] Invalid argument(s): error package:flutter_tools/src/test/font_config_manager.dart 43:7 FontConfigManager.dispose ===== asynchronous gap =========================== dart:async _CustomZone.registerBinaryCallback package:flutter_tools/src/test/font_config_manager.dart 38:9 FontConfigManager.dispose package:matcher expect test/general.shard/flutter_tester_device_test.dart 59:5 main.<fn> test/src/context.dart 141:42 testUsingContext.<fn>.<fn>.<fn>.<fn>.<fn> package:flutter_tools/src/base/context.dart 150:29 AppContext.run.<fn> dart:async runZoned package:flutter_tools/src/base/context.dart 149:12 AppContext.run test/src/context.dart 130:30 testUsingContext.<fn>.<fn>.<fn>.<fn> dart:async runZonedGuarded test/src/context.dart 128:18 testUsingContext.<fn>.<fn>.<fn> package:flutter_tools/src/base/context.dart 150:29 AppContext.run.<fn> dart:async runZoned package:flutter_tools/src/base/context.dart 149:12 AppContext.run test/src/context.dart 103:22 testUsingContext.<fn>.<fn> package:flutter_tools/src/context_runner.dart 84:18 runInContext.runnerWrapper ===== asynchronous gap =========================== dart:async _CustomZone.registerUnaryCallback package:flutter_tools/src/context_runner.dart 83:20 runInContext.runnerWrapper package:flutter_tools/src/base/context.dart 150:29 AppContext.run.<fn> dart:async runZoned package:flutter_tools/src/base/context.dart 149:12 AppContext.run package:flutter_tools/src/context_runner.dart 87:18 runInContext test/src/context.dart 102:11 testUsingContext.<fn> test/src/common.dart 183:18 test.<fn> To run this test again: /Users/eliasyishak/Desktop/flutter/bin/cache/dart-sdk/bin/dart test /Users/eliasyishak/Desktop/flutter/packages/flutter_tools/test/general.shard/flutter_tester_device_test.dart -p vm --plain-name 'Missing dir error caught for FontConfigManger.dispose' 00:01 +7 -1: Some tests failed. Oops; flutter has exited unexpectedly: "FileSystemException: error, path = ''". ^C Oops; flutter has exited unexpectedly: "FileSystemException: error, path = ''".

For both approaches, I have to ^C out of each process though....

I think the fact that uncaught exceptions hang the runner from running other tests is a bug with how we manage zones/context. consider this test:

import '../src/common.dart'; void main() { test('1', () => throw Exception('oops')); test('2', () {}); test('3', () {}); }

When I run this single threaded, it runs all tests, just failing the first, without hanging the runner at all.

But if I substitute testUsingContext, the first hangs and the rest of the tests don't run.

But if I substitute testUsingContext

Ah yep, thats it, I was assuming it would behave the same as test. That helps, thanks for the breakdown

This was good we narrowed it down to testUsingContext--we should fix this :)

Yeah I'll create an issue for this incase we don't have it already

christopherfujino

LGTM, thanks for the fix!

…tter/flutter#138496)

Roll Flutter from e8c2bb1 to 53a57ad (39 revisions) flutter/flutter@e8c2bb1...53a57ad 2023-11-16 [email protected] Roll Flutter Engine from 8ab189b77b8d to 2e9f0df868b3 (1 revision) (flutter/flutter#138543) 2023-11-16 [email protected] Roll Flutter Engine from 622fa0614412 to 8ab189b77b8d (1 revision) (flutter/flutter#138533) 2023-11-16 [email protected] [flutter_tools] - Add `queries` section to Android manifest file (flutter/flutter#137207) 2023-11-16 [email protected] Roll Flutter Engine from 8aff9c134b8f to 622fa0614412 (1 revision) (flutter/flutter#138532) 2023-11-16 [email protected] Roll Flutter Engine from 3cfcdebe8623 to 8aff9c134b8f (18 revisions) (flutter/flutter#138529) 2023-11-16 [email protected] Roll Flutter Engine from 30327eae0802 to 3cfcdebe8623 (1 revision) (flutter/flutter#138515) 2023-11-15 [email protected] Catch error for missing directory in `FontConfigManager` (flutter/flutter#138496) 2023-11-15 [email protected] Make `UnderlineInputBorder` consistent (flutter/flutter#124153) 2023-11-15 [email protected] Prepare `ShortcutActivator` and `ShortcutManager` to migrate to `KeyEvent` from `RawKeyEvent`. (flutter/flutter#136854) 2023-11-15 [email protected] Pin package:web 0.4.0 (flutter/flutter#138428) 2023-11-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland VelocityTracker update (#132291)" (flutter/flutter#138512) 2023-11-15 [email protected] [web] skip flaky overflow_clipbehavior_none.cupertino.0.png golden check (flutter/flutter#138498) 2023-11-15 [email protected] Roll Flutter Engine from 7c2b8d520b3d to 30327eae0802 (2 revisions) (flutter/flutter#138502) 2023-11-15 [email protected] Reland VelocityTracker update (#132291) (flutter/flutter#137381) 2023-11-15 [email protected] Roll Flutter Engine from f58dac64ad1a to 7c2b8d520b3d (1 revision) (flutter/flutter#138499) 2023-11-15 [email protected] Fix 2D tap to stop scrolling (flutter/flutter#138442) 2023-11-15 [email protected] Roll Flutter Engine from d22d063ac9f6 to f58dac64ad1a (2 revisions) (flutter/flutter#138494) 2023-11-15 [email protected] SemanticOwner should dispatch creation and disposal events (flutter/flutter#138388) 2023-11-15 [email protected] Roll Flutter Engine from ecaf9442034d to d22d063ac9f6 (5 revisions) (flutter/flutter#138489) 2023-11-15 [email protected] Roll Packages from 428ba3e to 0cd2378 (1 revision) (flutter/flutter#138482) 2023-11-15 [email protected] Marks Mac_android hot_mode_dev_cycle__benchmark to be unflaky (flutter/flutter#138472) 2023-11-15 [email protected] Roll Flutter Engine from a7a48a68e6f8 to ecaf9442034d (1 revision) (flutter/flutter#138468) 2023-11-15 [email protected] Roll Flutter Engine from 976edd5192d1 to a7a48a68e6f8 (3 revisions) (flutter/flutter#138463) 2023-11-15 [email protected] Roll Flutter Engine from a7f2267dd1f4 to 976edd5192d1 (1 revision) (flutter/flutter#138462) 2023-11-15 [email protected] Roll Flutter Engine from bc5bbd3b9ebe to a7f2267dd1f4 (1 revision) (flutter/flutter#138459) 2023-11-15 [email protected] Roll Flutter Engine from d7ca057b891f to bc5bbd3b9ebe (2 revisions) (flutter/flutter#138457) 2023-11-15 [email protected] Roll Flutter Engine from 1347413470b7 to d7ca057b891f (1 revision) (flutter/flutter#138456) 2023-11-15 [email protected] Roll Flutter Engine from c5a067b637f4 to 1347413470b7 (5 revisions) (flutter/flutter#138452) 2023-11-15 [email protected] Reland [SingleChildScrollView] Correct the offset pixels if it is out of range during layout (flutter/flutter#136871) 2023-11-14 [email protected] Add to TableCell docs (flutter/flutter#138258) 2023-11-14 [email protected] Roll Flutter Engine from f15b259fe98c to c5a067b637f4 (4 revisions) (flutter/flutter#138441) 2023-11-14 49699333+dependabot[bot]@users.noreply.github.com Bump dessant/lock-threads from 4.0.1 to 5.0.0 (flutter/flutter#138437) 2023-11-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.5 to 2.22.6 (flutter/flutter#138438) 2023-11-14 [email protected] Roll Flutter Engine from eba757803a6f to f15b259fe98c (1 revision) (flutter/flutter#138429) 2023-11-14 [email protected] Roll Flutter Engine from 603bdd48df8a to eba757803a6f (3 revisions) (flutter/flutter#138425) 2023-11-14 [email protected] Unified analytics migration for `CodeSizeAnalysis` (flutter/flutter#138351) 2023-11-14 [email protected] Roll Flutter Engine from 777dcb99f6e0 to 603bdd48df8a (1 revision) (flutter/flutter#138421) 2023-11-14 [email protected] Run all tests in examples/ (flutter/flutter#138374) 2023-11-14 [email protected] Roll Flutter Engine from 1b3fd80812c3 to 777dcb99f6e0 (2 revisions) (flutter/flutter#138420) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. ...

) Closes: - flutter#138434 We will catch any errors while attempting to clear the temp directories that don't exist for the `FontConfigManager` class

… (#139743) Closes: - #138434 We will catch any errors while attempting to clear the temp directories that don't exist for the `FontConfigManager` class

* [CP] Gold fix for stable branch (flutter#139764) Fixes flutter#139673 Cherry picks flutter#139706 to the stable branch to fix the tree. * [macOS] Suppress Xcode 15 createItemModels warning (flutter#138243) (flutter#139782) As of Xcode 15 on macOS Sonoma, the following message is (repeatedly) output to stderr during builds (repros on non-Flutter apps). It is supppressed in xcode itself, but not when run from the command-line. ``` 2023-11-10 10:44:58.031 xcodebuild[61115:1017566] [MT] DVTAssertions: Warning in /System/Volumes/Data/SWE/Apps/DT/BuildRoots/BuildRoot11/ActiveBuildRoot/Library/Caches/com.apple.xbs/Sources/IDEFrameworks/IDEFrameworks-22267/IDEFoundation/Provisioning/Capabilities Infrastructure/IDECapabilityQuerySelection.swift:103 Details: createItemModels creation requirements should not create capability item model for a capability item model that already exists. Function: createItemModels(for:itemModelSource:) Thread: <_NSMainThread: 0x6000027c0280>{number = 1, name = main} Please file a bug at https://feedbackassistant.apple.com with this warning message and any useful information you can provide. ``` This suppresses this message from stderr in our macOS build logs. Issue: flutter#135277 Cherry-pick: flutter#139284 https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat *Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.* *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* * [CP] have `Java.version` return null if `java --version` fails or cannot be run (flutter#139698) cherry-picks changes from flutter#139614 onto the stable channel * [CP] Catch error for missing directory in `FontConfigManager` (flutter#138496) (flutter#139743) Closes: - flutter#138434 We will catch any errors while attempting to clear the temp directories that don't exist for the `FontConfigManager` class --------- Co-authored-by: Kate Lovett <[email protected]> Co-authored-by: Chris Bracken <[email protected]> Co-authored-by: Andrew Kolos <[email protected]> Co-authored-by: Elias Yishak <[email protected]>

…tter/flutter#138496)

Catch error for missing directory

e37f0d2

eliasyishak added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 15, 2023

eliasyishak requested a review from christopherfujino November 15, 2023 17:46

eliasyishak changed the title ~~Catch error for missing directory~~ Catch error for missing directory in FontConfigManager Nov 15, 2023

christopherfujino reviewed Nov 15, 2023

View reviewed changes

packages/flutter_tools/lib/src/test/font_config_manager.dart Outdated Show resolved Hide resolved

Catch more specific exception

936ef44

christopherfujino reviewed Nov 15, 2023

View reviewed changes

Invoke dispose instead of wrapping in expect

53361f0

christopherfujino approved these changes Nov 15, 2023

View reviewed changes

eliasyishak mentioned this pull request Nov 15, 2023

Tests with testUsingContext hang when error is thrown #138510

Closed

eliasyishak added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 15, 2023

auto-submit bot merged commit 04afff3 into flutter:master Nov 15, 2023

engine-flutter-autoroll mentioned this pull request Nov 16, 2023

Roll Flutter from e8c2bb135659 to 53a57adcf136 (39 revisions) flutter/packages#5412

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 16, 2023

04afff3 Catch error for missing directory in FontConfigManager (flu…

ad11065

…tter/flutter#138496)

eliasyishak mentioned this pull request Nov 16, 2023

[flutter_tools] PathNotFoundException during FontConfigManager.dispose #138434

Closed

eliasyishak deleted the 138434-flutter_tools-pathnotfoundexception branch November 16, 2023 17:14

eliasyishak restored the 138434-flutter_tools-pathnotfoundexception branch December 7, 2023 16:34

eliasyishak mentioned this pull request Dec 7, 2023

[CP] Catch error for missing directory in FontConfigManager #139742

Closed

eliasyishak deleted the 138434-flutter_tools-pathnotfoundexception branch December 12, 2023 21:35

eliasyishak restored the 138434-flutter_tools-pathnotfoundexception branch December 12, 2023 22:03

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024

04afff3 Catch error for missing directory in FontConfigManager (flu…

fecac3d

…tter/flutter#138496)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Catch error for missing directory in `FontConfigManager` #138496

Catch error for missing directory in `FontConfigManager` #138496

Uh oh!

eliasyishak commented Nov 15, 2023

Uh oh!

Uh oh!

christopherfujino Nov 15, 2023

Uh oh!

eliasyishak Nov 15, 2023

Uh oh!

christopherfujino Nov 15, 2023 •

edited

Loading

Uh oh!

eliasyishak Nov 15, 2023

Uh oh!

eliasyishak Nov 15, 2023

Uh oh!

christopherfujino Nov 15, 2023

Uh oh!

christopherfujino Nov 15, 2023

Uh oh!

eliasyishak Nov 15, 2023

Uh oh!

christopherfujino Nov 15, 2023

Uh oh!

eliasyishak Nov 15, 2023

Uh oh!

christopherfujino left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Catch error for missing directory in FontConfigManager #138496

Catch error for missing directory in FontConfigManager #138496

Uh oh!

Conversation

eliasyishak commented Nov 15, 2023

Pre-launch Checklist

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino Nov 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Catch error for missing directory in `FontConfigManager` #138496

Catch error for missing directory in `FontConfigManager` #138496

christopherfujino Nov 15, 2023 •

edited

Loading