From b04adb99e54228c53d753df603f228f07baa282f Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Mon, 7 Jun 2021 12:59:23 -0700 Subject: [PATCH 1/2] Refactor lib/dartdoc and dartdoc_test --- lib/dartdoc.dart | 48 ++++++++--------- test/end2end/dartdoc_test.dart | 96 +++++++++++++++------------------- 2 files changed, 67 insertions(+), 77 deletions(-) diff --git a/lib/dartdoc.dart b/lib/dartdoc.dart index 48ddb4bc1a..b8bb3dcf87 100644 --- a/lib/dartdoc.dart +++ b/lib/dartdoc.dart @@ -29,6 +29,7 @@ import 'package:dartdoc/src/utils.dart'; import 'package:dartdoc/src/version.dart'; import 'package:dartdoc/src/warnings.dart'; import 'package:html/parser.dart' show parse; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; export 'package:dartdoc/src/dartdoc_options.dart'; @@ -43,14 +44,14 @@ const String programName = 'dartdoc'; const String dartdocVersion = packageVersion; class DartdocFileWriter implements FileWriter { - final String outputDir; + final String _outputDir; @override final ResourceProvider resourceProvider; final Map _fileElementMap = {}; @override final Set writtenFiles = {}; - DartdocFileWriter(this.outputDir, this.resourceProvider); + DartdocFileWriter(this._outputDir, this.resourceProvider); @override void writeBytes( @@ -97,11 +98,11 @@ class DartdocFileWriter implements FileWriter { } } - /// Returns the file at [outFile] relative to [outputDir], creating the parent - /// directory if necessary. + /// Returns the file at [outFile] relative to [_outputDir], creating the + /// parent directory if necessary. File _getFile(String outFile) { var file = resourceProvider - .getFile(resourceProvider.pathContext.join(outputDir, outFile)); + .getFile(resourceProvider.pathContext.join(_outputDir, outFile)); var parent = file.parent2; if (!parent.exists) { parent.create(); @@ -116,15 +117,15 @@ class Dartdoc { final Generator generator; final PackageBuilder packageBuilder; final DartdocOptionContext config; - final Set writtenFiles = {}; - Folder outputDir; + final Set _writtenFiles = {}; + Folder _outputDir; // Fires when the self checks make progress. final StreamController _onCheckProgress = StreamController(sync: true); Dartdoc._(this.config, this.generator, this.packageBuilder) { - outputDir = config.resourceProvider + _outputDir = config.resourceProvider .getFolder(config.resourceProvider.pathContext.absolute(config.output)) ..create(); } @@ -183,16 +184,11 @@ class Dartdoc { PackageGraph packageGraph; - /// Generate Dartdoc documentation. - /// - /// [DartdocResults] is returned if dartdoc succeeds. [DartdocFailure] is - /// thrown if dartdoc fails in an expected way, for example if there is an - /// analysis error in the code. + @visibleForTesting Future generateDocsBase() async { var stopwatch = Stopwatch()..start(); - double seconds; packageGraph = await packageBuilder.buildPackageGraph(); - seconds = stopwatch.elapsedMilliseconds / 1000.0; + var seconds = stopwatch.elapsedMilliseconds / 1000.0; var libs = packageGraph.libraries.length; logInfo("Initialized dartdoc with $libs librar${libs == 1 ? 'y' : 'ies'} " 'in ${seconds.toStringAsFixed(1)} seconds'); @@ -201,14 +197,14 @@ class Dartdoc { var generator = this.generator; if (generator != null) { // Create the out directory. - if (!outputDir.exists) outputDir.create(); + if (!_outputDir.exists) _outputDir.create(); - var writer = DartdocFileWriter(outputDir.path, config.resourceProvider); + var writer = DartdocFileWriter(_outputDir.path, config.resourceProvider); await generator.generate(packageGraph, writer); - writtenFiles.addAll(writer.writtenFiles); - if (config.validateLinks && writtenFiles.isNotEmpty) { - validateLinks(packageGraph, outputDir.path); + _writtenFiles.addAll(writer.writtenFiles); + if (config.validateLinks && _writtenFiles.isNotEmpty) { + _validateLinks(packageGraph, _outputDir.path); } } @@ -229,9 +225,14 @@ class Dartdoc { if (config.showStats) { logInfo(markdownStats.buildReport()); } - return DartdocResults(config.topLevelPackageMeta, packageGraph, outputDir); + return DartdocResults(config.topLevelPackageMeta, packageGraph, _outputDir); } + /// Generate Dartdoc documentation. + /// + /// [DartdocResults] is returned if dartdoc succeeds. [DartdocFailure] is + /// thrown if dartdoc fails in an expected way, for example if there is an + /// analysis error in the code. Future generateDocs() async { try { logInfo('Documenting ${config.topLevelPackageMeta}...'); @@ -252,7 +253,6 @@ class Dartdoc { return dartdocResults; } finally { // Clear out any cached tool snapshots and temporary directories. - // ignore: unawaited_futures SnapshotCache.instance?.dispose(); // ignore: unawaited_futures ToolTempFileTracker.instance?.dispose(); @@ -325,7 +325,7 @@ class Dartdoc { } if (visited.contains(fullPath)) continue; var relativeFullPath = path.relative(fullPath, from: normalOrigin); - if (!writtenFiles.contains(relativeFullPath)) { + if (!_writtenFiles.contains(relativeFullPath)) { // This isn't a file we wrote (this time); don't claim we did. _warn( packageGraph, PackageWarning.unknownFile, fullPath, normalOrigin); @@ -471,7 +471,7 @@ class Dartdoc { /// Don't call this method more than once, and only after you've /// generated all docs for the Package. - void validateLinks(PackageGraph packageGraph, String origin) { + void _validateLinks(PackageGraph packageGraph, String origin) { assert(_hrefs == null); _hrefs = packageGraph.allHrefs; diff --git a/test/end2end/dartdoc_test.dart b/test/end2end/dartdoc_test.dart index e800e2b92d..ac26b99706 100644 --- a/test/end2end/dartdoc_test.dart +++ b/test/end2end/dartdoc_test.dart @@ -68,7 +68,6 @@ class DartdocLoggingOptionContext extends DartdocGeneratorOptionContext void main() { group('dartdoc with generators', () { - var resourceProvider = pubPackageMetaProvider.resourceProvider; Folder tempDir; setUpAll(() async { @@ -77,20 +76,19 @@ void main() { optionSet.parseArguments([]); startLogging(DartdocLoggingOptionContext( optionSet, - resourceProvider.getFolder(resourceProvider.pathContext.current), - resourceProvider)); + _resourceProvider.getFolder(_pathContext.current), + _resourceProvider)); }); setUp(() async { - tempDir = resourceProvider.createSystemTemp('dartdoc.test.'); + tempDir = _resourceProvider.createSystemTemp('dartdoc.test.'); }); tearDown(() async { tempDir.delete(); }); - Future buildDartdoc( - List argv, Folder packageRoot, Folder tempDir) async { + Future buildDartdoc(List argv, Folder packageRoot) async { var context = await _generatorContextFromArgv(argv ..addAll(['--input', packageRoot.path, '--output', tempDir.path])); @@ -102,23 +100,20 @@ void main() { } group('Option handling', () { - Dartdoc dartdoc; - DartdocResults results; PackageGraph p; - Folder tempDir; setUpAll(() async { - tempDir = resourceProvider.createSystemTemp('dartdoc.test.'); - dartdoc = await buildDartdoc([], _testPackageOptions, tempDir); - results = await dartdoc.generateDocsBase(); + tempDir = _resourceProvider.createSystemTemp('dartdoc.test.'); + var dartdoc = await buildDartdoc([], _testPackageOptions); + var results = await dartdoc.generateDocsBase(); p = results.packageGraph; }); test('generator parameters', () async { - var favicon = resourceProvider.getFile(resourceProvider.pathContext + var favicon = _resourceProvider.getFile(_pathContext .joinAll([tempDir.path, 'static-assets', 'favicon.png'])); - var index = resourceProvider.getFile( - resourceProvider.pathContext.joinAll([tempDir.path, 'index.html'])); + var index = _resourceProvider + .getFile(_pathContext.joinAll([tempDir.path, 'index.html'])); expect(favicon.readAsStringSync(), contains('Not really a png, but a test file')); var indexString = index.readAsStringSync(); @@ -147,8 +142,7 @@ void main() { }); test('errors generate errors even when warnings are off', () async { - var dartdoc = - await buildDartdoc(['--allow-tools'], testPackageToolError, tempDir); + var dartdoc = await buildDartdoc(['--allow-tools'], testPackageToolError); var results = await dartdoc.generateDocsBase(); var p = results.packageGraph; var unresolvedToolErrors = p.packageWarningCounter.countedWarnings.values @@ -162,8 +156,7 @@ void main() { }); test('with broken reexport chain', () async { - var dartdoc = - await buildDartdoc([], _testPackageImportExportError, tempDir); + var dartdoc = await buildDartdoc([], _testPackageImportExportError); var results = await dartdoc.generateDocsBase(); var p = results.packageGraph; var unresolvedExportWarnings = p @@ -178,8 +171,7 @@ void main() { group('include/exclude parameters', () { test('with config file', () async { - var dartdoc = - await buildDartdoc([], _testPackageIncludeExclude, tempDir); + var dartdoc = await buildDartdoc([], _testPackageIncludeExclude); var results = await dartdoc.generateDocs(); var p = results.packageGraph; expect(p.localPublicLibraries.map((l) => l.name), @@ -187,8 +179,8 @@ void main() { }); test('with include command line argument', () async { - var dartdoc = await buildDartdoc(['--include', 'another_included'], - _testPackageIncludeExclude, tempDir); + var dartdoc = await buildDartdoc( + ['--include', 'another_included'], _testPackageIncludeExclude); var results = await dartdoc.generateDocs(); var p = results.packageGraph; expect(p.localPublicLibraries.length, equals(1)); @@ -196,8 +188,8 @@ void main() { }); test('with exclude command line argument', () async { - var dartdoc = await buildDartdoc(['--exclude', 'more_included'], - _testPackageIncludeExclude, tempDir); + var dartdoc = await buildDartdoc( + ['--exclude', 'more_included'], _testPackageIncludeExclude); var results = await dartdoc.generateDocs(); var p = results.packageGraph; expect(p.localPublicLibraries.length, equals(1)); @@ -207,8 +199,8 @@ void main() { }); test('basic interlinking test', () async { - var dartdoc = await buildDartdoc( - ['--exclude-packages=args'], _testPackageDir, tempDir); + var dartdoc = + await buildDartdoc(['--exclude-packages=args'], _testPackageDir); var results = await dartdoc.generateDocs(); var p = results.packageGraph; var meta = p.publicPackages.firstWhere((p) => p.name == 'meta'); @@ -233,13 +225,11 @@ void main() { }); group('validate basic doc generation', () { - Dartdoc dartdoc; DartdocResults results; - Folder tempDir; setUpAll(() async { - tempDir = resourceProvider.createSystemTemp('dartdoc.test.'); - dartdoc = await buildDartdoc([], _testPackageDir, tempDir); + tempDir = _resourceProvider.createSystemTemp('dartdoc.test.'); + var dartdoc = await buildDartdoc([], _testPackageDir); results = await dartdoc.generateDocs(); }); @@ -262,9 +252,10 @@ void main() { test('source code links are visible', () async { // Picked this object as this library explicitly should never contain // a library directive, so we can predict what line number it will be. - var anonymousOutput = resourceProvider.getFile( - resourceProvider.pathContext.join(tempDir.path, 'anonymous_library', - 'anonymous_library-library.html')); + var anonymousOutput = _resourceProvider.getFile(_pathContext.join( + tempDir.path, + 'anonymous_library', + 'anonymous_library-library.html')); expect(anonymousOutput.exists, isTrue); expect( anonymousOutput.readAsStringSync(), @@ -276,7 +267,7 @@ void main() { test('generate docs for ${path.basename(_testPackageBadDir.path)} fails', () async { - var dartdoc = await buildDartdoc([], _testPackageBadDir, tempDir); + var dartdoc = await buildDartdoc([], _testPackageBadDir); try { await dartdoc.generateDocs(); @@ -289,7 +280,7 @@ void main() { 'from analysis_options'); test('generate docs for package with embedder yaml', () async { - var dartdoc = await buildDartdoc([], _testSkyEnginePackage, tempDir); + var dartdoc = await buildDartdoc([], _testSkyEnginePackage); var results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); @@ -324,8 +315,8 @@ void main() { test('generate docs with custom templates', () async { var templatesDir = path.join(_testPackageCustomTemplates.path, 'templates'); - var dartdoc = await buildDartdoc(['--templates-dir', templatesDir], - _testPackageCustomTemplates, tempDir); + var dartdoc = await buildDartdoc( + ['--templates-dir', templatesDir], _testPackageCustomTemplates); var results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); @@ -334,8 +325,8 @@ void main() { expect(p.defaultPackage.name, 'test_package_custom_templates'); expect(p.localPublicLibraries, hasLength(1)); - var index = resourceProvider.getFile( - resourceProvider.pathContext.join(tempDir.path, 'index.html')); + var index = _resourceProvider + .getFile(_pathContext.join(tempDir.path, 'index.html')); expect(index.readAsStringSync(), contains('Welcome my friends to a custom template')); }); @@ -343,8 +334,8 @@ void main() { test('generate docs with missing required template fails', () async { var templatesDir = path.join(path.current, 'test/templates'); try { - await buildDartdoc(['--templates-dir', templatesDir], - _testPackageCustomTemplates, tempDir); + await buildDartdoc( + ['--templates-dir', templatesDir], _testPackageCustomTemplates); fail('dartdoc should fail with missing required template'); } catch (e) { expect(e is DartdocFailure, isTrue); @@ -357,7 +348,7 @@ void main() { var badPath = path.join(tempDir.path, 'BAD'); try { await buildDartdoc( - ['--templates-dir', badPath], _testPackageCustomTemplates, tempDir); + ['--templates-dir', badPath], _testPackageCustomTemplates); fail('dartdoc should fail with bad templatesDir path'); } catch (e) { expect(e is DartdocFailure, isTrue); @@ -365,34 +356,33 @@ void main() { }); test('generating markdown docs does not crash', () async { - var dartdoc = - await buildDartdoc(['--format', 'md'], _testPackageDir, tempDir); + var dartdoc = await buildDartdoc(['--format', 'md'], _testPackageDir); await dartdoc.generateDocsBase(); }); test('generating markdown docs for experimental features does not crash', () async { - var dartdoc = await buildDartdoc( - ['--format', 'md'], _testPackageExperiments, tempDir); + var dartdoc = + await buildDartdoc(['--format', 'md'], _testPackageExperiments); await dartdoc.generateDocsBase(); }, skip: !_experimentPackageAllowed.allows(platformVersion)); test('rel canonical prefix does not include base href', () async { final prefix = 'foo.bar/baz'; var dartdoc = await buildDartdoc( - ['--rel-canonical-prefix', prefix], _testPackageDir, tempDir); + ['--rel-canonical-prefix', prefix], _testPackageDir); await dartdoc.generateDocsBase(); // Verify files at different levels have correct content. - var level1 = resourceProvider.getFile(resourceProvider.pathContext - .join(tempDir.path, 'ex', 'Apple-class.html')); + var level1 = _resourceProvider + .getFile(_pathContext.join(tempDir.path, 'ex', 'Apple-class.html')); expect(level1.exists, isTrue); expect( level1.readAsStringSync(), contains( '')); - var level2 = resourceProvider.getFile(resourceProvider.pathContext - .join(tempDir.path, 'ex', 'Apple', 'm.html')); + var level2 = _resourceProvider + .getFile(_pathContext.join(tempDir.path, 'ex', 'Apple', 'm.html')); expect(level2.exists, isTrue); expect(level2.readAsStringSync(), contains('')); @@ -400,7 +390,7 @@ void main() { test('generate docs with bad output format', () async { try { - await buildDartdoc(['--format', 'bad'], _testPackageDir, tempDir); + await buildDartdoc(['--format', 'bad'], _testPackageDir); fail('dartdoc should fail with bad output format'); } catch (e) { expect(e is DartdocFailure, isTrue); From 741fe732e3fdcf232fb8dcb250dbd7003d6524a1 Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Mon, 7 Jun 2021 13:21:42 -0700 Subject: [PATCH 2/2] Put tempDir back --- test/end2end/dartdoc_test.dart | 59 ++++++++++++++++++++-------------- 1 file changed, 34 insertions(+), 25 deletions(-) diff --git a/test/end2end/dartdoc_test.dart b/test/end2end/dartdoc_test.dart index ac26b99706..91ccfc83f1 100644 --- a/test/end2end/dartdoc_test.dart +++ b/test/end2end/dartdoc_test.dart @@ -88,7 +88,8 @@ void main() { tempDir.delete(); }); - Future buildDartdoc(List argv, Folder packageRoot) async { + Future buildDartdoc( + List argv, Folder packageRoot, Folder tempDir) async { var context = await _generatorContextFromArgv(argv ..addAll(['--input', packageRoot.path, '--output', tempDir.path])); @@ -100,12 +101,15 @@ void main() { } group('Option handling', () { + Dartdoc dartdoc; + DartdocResults results; PackageGraph p; + Folder tempDir; setUpAll(() async { tempDir = _resourceProvider.createSystemTemp('dartdoc.test.'); - var dartdoc = await buildDartdoc([], _testPackageOptions); - var results = await dartdoc.generateDocsBase(); + dartdoc = await buildDartdoc([], _testPackageOptions, tempDir); + results = await dartdoc.generateDocsBase(); p = results.packageGraph; }); @@ -142,7 +146,8 @@ void main() { }); test('errors generate errors even when warnings are off', () async { - var dartdoc = await buildDartdoc(['--allow-tools'], testPackageToolError); + var dartdoc = + await buildDartdoc(['--allow-tools'], testPackageToolError, tempDir); var results = await dartdoc.generateDocsBase(); var p = results.packageGraph; var unresolvedToolErrors = p.packageWarningCounter.countedWarnings.values @@ -156,7 +161,8 @@ void main() { }); test('with broken reexport chain', () async { - var dartdoc = await buildDartdoc([], _testPackageImportExportError); + var dartdoc = + await buildDartdoc([], _testPackageImportExportError, tempDir); var results = await dartdoc.generateDocsBase(); var p = results.packageGraph; var unresolvedExportWarnings = p @@ -171,7 +177,8 @@ void main() { group('include/exclude parameters', () { test('with config file', () async { - var dartdoc = await buildDartdoc([], _testPackageIncludeExclude); + var dartdoc = + await buildDartdoc([], _testPackageIncludeExclude, tempDir); var results = await dartdoc.generateDocs(); var p = results.packageGraph; expect(p.localPublicLibraries.map((l) => l.name), @@ -179,8 +186,8 @@ void main() { }); test('with include command line argument', () async { - var dartdoc = await buildDartdoc( - ['--include', 'another_included'], _testPackageIncludeExclude); + var dartdoc = await buildDartdoc(['--include', 'another_included'], + _testPackageIncludeExclude, tempDir); var results = await dartdoc.generateDocs(); var p = results.packageGraph; expect(p.localPublicLibraries.length, equals(1)); @@ -188,8 +195,8 @@ void main() { }); test('with exclude command line argument', () async { - var dartdoc = await buildDartdoc( - ['--exclude', 'more_included'], _testPackageIncludeExclude); + var dartdoc = await buildDartdoc(['--exclude', 'more_included'], + _testPackageIncludeExclude, tempDir); var results = await dartdoc.generateDocs(); var p = results.packageGraph; expect(p.localPublicLibraries.length, equals(1)); @@ -199,8 +206,8 @@ void main() { }); test('basic interlinking test', () async { - var dartdoc = - await buildDartdoc(['--exclude-packages=args'], _testPackageDir); + var dartdoc = await buildDartdoc( + ['--exclude-packages=args'], _testPackageDir, tempDir); var results = await dartdoc.generateDocs(); var p = results.packageGraph; var meta = p.publicPackages.firstWhere((p) => p.name == 'meta'); @@ -226,10 +233,11 @@ void main() { group('validate basic doc generation', () { DartdocResults results; + Folder tempDir; setUpAll(() async { tempDir = _resourceProvider.createSystemTemp('dartdoc.test.'); - var dartdoc = await buildDartdoc([], _testPackageDir); + var dartdoc = await buildDartdoc([], _testPackageDir, tempDir); results = await dartdoc.generateDocs(); }); @@ -267,7 +275,7 @@ void main() { test('generate docs for ${path.basename(_testPackageBadDir.path)} fails', () async { - var dartdoc = await buildDartdoc([], _testPackageBadDir); + var dartdoc = await buildDartdoc([], _testPackageBadDir, tempDir); try { await dartdoc.generateDocs(); @@ -280,7 +288,7 @@ void main() { 'from analysis_options'); test('generate docs for package with embedder yaml', () async { - var dartdoc = await buildDartdoc([], _testSkyEnginePackage); + var dartdoc = await buildDartdoc([], _testSkyEnginePackage, tempDir); var results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); @@ -315,8 +323,8 @@ void main() { test('generate docs with custom templates', () async { var templatesDir = path.join(_testPackageCustomTemplates.path, 'templates'); - var dartdoc = await buildDartdoc( - ['--templates-dir', templatesDir], _testPackageCustomTemplates); + var dartdoc = await buildDartdoc(['--templates-dir', templatesDir], + _testPackageCustomTemplates, tempDir); var results = await dartdoc.generateDocs(); expect(results.packageGraph, isNotNull); @@ -334,8 +342,8 @@ void main() { test('generate docs with missing required template fails', () async { var templatesDir = path.join(path.current, 'test/templates'); try { - await buildDartdoc( - ['--templates-dir', templatesDir], _testPackageCustomTemplates); + await buildDartdoc(['--templates-dir', templatesDir], + _testPackageCustomTemplates, tempDir); fail('dartdoc should fail with missing required template'); } catch (e) { expect(e is DartdocFailure, isTrue); @@ -348,7 +356,7 @@ void main() { var badPath = path.join(tempDir.path, 'BAD'); try { await buildDartdoc( - ['--templates-dir', badPath], _testPackageCustomTemplates); + ['--templates-dir', badPath], _testPackageCustomTemplates, tempDir); fail('dartdoc should fail with bad templatesDir path'); } catch (e) { expect(e is DartdocFailure, isTrue); @@ -356,21 +364,22 @@ void main() { }); test('generating markdown docs does not crash', () async { - var dartdoc = await buildDartdoc(['--format', 'md'], _testPackageDir); + var dartdoc = + await buildDartdoc(['--format', 'md'], _testPackageDir, tempDir); await dartdoc.generateDocsBase(); }); test('generating markdown docs for experimental features does not crash', () async { - var dartdoc = - await buildDartdoc(['--format', 'md'], _testPackageExperiments); + var dartdoc = await buildDartdoc( + ['--format', 'md'], _testPackageExperiments, tempDir); await dartdoc.generateDocsBase(); }, skip: !_experimentPackageAllowed.allows(platformVersion)); test('rel canonical prefix does not include base href', () async { final prefix = 'foo.bar/baz'; var dartdoc = await buildDartdoc( - ['--rel-canonical-prefix', prefix], _testPackageDir); + ['--rel-canonical-prefix', prefix], _testPackageDir, tempDir); await dartdoc.generateDocsBase(); // Verify files at different levels have correct content. @@ -390,7 +399,7 @@ void main() { test('generate docs with bad output format', () async { try { - await buildDartdoc(['--format', 'bad'], _testPackageDir); + await buildDartdoc(['--format', 'bad'], _testPackageDir, tempDir); fail('dartdoc should fail with bad output format'); } catch (e) { expect(e is DartdocFailure, isTrue);