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

Update all (approximately) i/o operations to use ResourceProvider #2315

Merged
merged 12 commits into from
Aug 31, 2020

Conversation

srawlins
Copy link
Member

@srawlins srawlins commented Aug 20, 2020

Fixes #2297

All i/o operations now pass through a ResourceProvider, an abstraction which allows disk-free testing. In this change, almost all ResourceProviders used are the PhysicalResourceProvider; no tests are converted from being filesystem-dependent to using MemoryResourceProvider.

dartdoc uses the filesystem extensively, so this change is large. Often top-level or static elements become more complicated, depending on a ResourceProvider variable.

Some extra dart:io functionality not found in ResourceProvider is added via an extension, ResourceProviderExtensions.

Many classes have a new ResourceProvider resourceProvider field: DartdocFileWriter, SnapshotCache, DartToolDefinition, ToolConfiguration, DartdocOption, PackageMetaProvider, PackageMeta, ToolTempFileTracker,

Each of SnapshotCache and ToolTempFileTracker has a static instance field which took no arguments; now that they need a ResourceProvider, it was unwieldy to pass a ResourceProvider to get the instance each time, so a new method, createInstance creates the instance.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Aug 20, 2020
@srawlins srawlins requested a review from scheglov August 20, 2020 17:17
lib/dartdoc.dart Outdated

void checkDirectory(Folder dir) {
for (var f in dir.getChildren()) {
//if (path.basename(f.path) == '.' || path.basename(f.path) == '..') {
Copy link
Member

Choose a reason for hiding this comment

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

Do you intend this to be commented out?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, fixed.

if (this is PhysicalResourceProvider) {
return io.Platform.resolvedExecutable;
} else {
return 'TODO(srawlins)';
Copy link
Member

Choose a reason for hiding this comment

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

I assume this PR is still a WIP?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, that part is not implemented yet. It will be the part used in tests with MemoryResourceProvider. Moved TODO to comment; return null.

@srawlins
Copy link
Member Author

Same TargetKind crash on analyzer at HEAD. Side-stepping.

@srawlins
Copy link
Member Author

Side-stepping analyzer 0.40.1 breakage.

@srawlins srawlins merged commit a6900d9 into dart-lang:master Aug 31, 2020
@srawlins srawlins deleted the resource-provider branch August 31, 2020 01:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a file system abstraction
4 participants