From 0d564c77a1d4accfde1d46f3bbe2331f071c31da Mon Sep 17 00:00:00 2001 From: Sam Rawlins Date: Sat, 3 Oct 2020 16:06:57 -0700 Subject: [PATCH] Migrate html_generator_test to be a unit test. This also required making ModelNode use ResourceProvider. Additionally, extracted some common utilities from html_generator_test, library_test, and package_test, into utils. --- lib/src/model/model_node.dart | 7 +- lib/src/model/package_graph.dart | 4 +- lib/src/model_utils.dart | 6 +- test/end2end/html_generator_test.dart | 169 ------------------ test/html_generator_test.dart | 131 ++++++++++++++ test/library_test.dart | 28 +-- test/package_test.dart | 24 +-- test/src/utils.dart | 26 +++ testing/test_package_duplicate/lib/first.dart | 1 - .../test_package_duplicate/lib/second.dart | 1 - testing/test_package_duplicate/pubspec.yaml | 1 - 11 files changed, 175 insertions(+), 223 deletions(-) delete mode 100644 test/end2end/html_generator_test.dart create mode 100644 test/html_generator_test.dart delete mode 100644 testing/test_package_duplicate/lib/first.dart delete mode 100644 testing/test_package_duplicate/lib/second.dart delete mode 100644 testing/test_package_duplicate/pubspec.yaml diff --git a/lib/src/model/model_node.dart b/lib/src/model/model_node.dart index 6ba32f4772..15d0fa39a0 100644 --- a/lib/src/model/model_node.dart +++ b/lib/src/model/model_node.dart @@ -4,6 +4,7 @@ import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/file_system/file_system.dart'; import 'package:dartdoc/src/model_utils.dart' as model_utils; /// A stripped down [CommentReference] containing only that information needed @@ -22,11 +23,12 @@ class ModelCommentReference { class ModelNode { final List commentRefs; final Element element; + final ResourceProvider resourceProvider; final int _sourceOffset; final int _sourceEnd; - ModelNode(AstNode sourceNode, this.element) + ModelNode(AstNode sourceNode, this.element, this.resourceProvider) : _sourceOffset = sourceNode?.offset, _sourceEnd = sourceNode?.end, commentRefs = _commentRefsFor(sourceNode); @@ -46,7 +48,8 @@ class ModelNode { String get sourceCode { if (_sourceCode == null) { if (_sourceOffset != null) { - var contents = model_utils.getFileContentsFor(element); + var contents = + model_utils.getFileContentsFor(element, resourceProvider); // Find the start of the line, so that we can line up all the indents. var i = _sourceOffset; while (i > 0) { diff --git a/lib/src/model/package_graph.dart b/lib/src/model/package_graph.dart index bc48c8cca0..ba3d9a9e58 100644 --- a/lib/src/model/package_graph.dart +++ b/lib/src/model/package_graph.dart @@ -140,8 +140,8 @@ class PackageGraph { Element element, Map compilationUnitMap) { _modelNodes.putIfAbsent( element, - () => - ModelNode(utils.getAstNode(element, compilationUnitMap), element)); + () => ModelNode(utils.getAstNode(element, compilationUnitMap), element, + resourceProvider)); } ModelNode getModelNodeFor(Element element) => _modelNodes[element]; diff --git a/lib/src/model_utils.dart b/lib/src/model_utils.dart index 1c826d5ff8..f798b6c649 100644 --- a/lib/src/model_utils.dart +++ b/lib/src/model_utils.dart @@ -5,10 +5,10 @@ library dartdoc.model_utils; import 'dart:convert'; -import 'dart:io'; import 'package:analyzer/dart/ast/ast.dart'; import 'package:analyzer/dart/element/element.dart'; +import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/src/dart/ast/utilities.dart'; import 'package:dartdoc/src/model/model.dart'; @@ -50,10 +50,10 @@ Iterable findCanonicalFor(Iterable classes) { c.packageGraph.findCanonicalModelElementFor(c.element) as Class ?? c); } -String getFileContentsFor(Element e) { +String getFileContentsFor(Element e, ResourceProvider resourceProvider) { var location = e.source.fullName; if (!_fileContents.containsKey(location)) { - var contents = File(location).readAsStringSync(); + var contents = resourceProvider.getFile(location).readAsStringSync(); _fileContents.putIfAbsent(location, () => contents); } return _fileContents[location]; diff --git a/test/end2end/html_generator_test.dart b/test/end2end/html_generator_test.dart deleted file mode 100644 index d2b2bd1b8e..0000000000 --- a/test/end2end/html_generator_test.dart +++ /dev/null @@ -1,169 +0,0 @@ -// Copyright (c) 2015, the Dart project authors. Please see the AUTHORS file -// for details. All rights reserved. Use of this source code is governed by a -// BSD-style license that can be found in the LICENSE file. - -library dartdoc.html_generator_test; - -import 'package:analyzer/file_system/file_system.dart'; -import 'package:dartdoc/dartdoc.dart'; -import 'package:dartdoc/src/generator/generator_frontend.dart'; -import 'package:dartdoc/src/generator/html_generator.dart'; -import 'package:dartdoc/src/generator/html_resources.g.dart'; -import 'package:dartdoc/src/generator/templates.dart'; -import 'package:dartdoc/src/model/package_graph.dart'; -import 'package:dartdoc/src/io_utils.dart'; -import 'package:dartdoc/src/package_config_provider.dart'; -import 'package:dartdoc/src/warnings.dart'; -import 'package:path/path.dart' as path; -import 'package:test/test.dart'; - -import '../src/utils.dart' as utils; - -// Init a generator without a GeneratorContext and with the default file writer. -Future _initGeneratorForTest() async { - var backend = - HtmlGeneratorBackend(null, await Templates.createDefault('html')); - return GeneratorFrontEnd(backend); -} - -void main() { - group('Templates', () { - Templates templates; - - setUp(() async { - templates = await Templates.createDefault('html'); - }); - - test('index html', () { - expect(templates.indexTemplate, isNotNull); - }); - - test('library', () { - expect(templates.libraryTemplate, isNotNull); - }); - - test('class', () { - expect(templates.classTemplate, isNotNull); - }); - - test('function', () { - expect(templates.functionTemplate, isNotNull); - }); - - test('constructor', () { - expect(templates.constructorTemplate, isNotNull); - }); - - test('method', () { - expect(templates.methodTemplate, isNotNull); - }); - - test('constant', () { - expect(templates.constantTemplate, isNotNull); - }); - - test('property', () { - expect(templates.propertyTemplate, isNotNull); - }); - - test('top level constant', () { - expect(templates.topLevelConstantTemplate, isNotNull); - }); - - test('top level property', () { - expect(templates.topLevelPropertyTemplate, isNotNull); - }); - }); - - group('HtmlGenerator', () { - Generator generator; - var resourceProvider = pubPackageMetaProvider.resourceProvider; - var pathContext = resourceProvider.pathContext; - Folder tempOutput; - FileWriter writer; - - // TODO: Run the HtmlGenerator and validate important constraints. - group('for a null package', () { - setUp(() async { - generator = await _initGeneratorForTest(); - tempOutput = resourceProvider.createSystemTemp('doc_test_temp'); - writer = DartdocFileWriter(tempOutput.path, resourceProvider); - return generator.generate(null, writer); - }); - - tearDown(() { - if (tempOutput != null) { - tempOutput.delete(); - } - }); - - test('resources are put into the right place', () { - var output = resourceProvider - .getFolder(pathContext.join(tempOutput.path, 'static-assets')); - expect(output, doesExist); - - for (var resource in resource_names.map((r) => - path.relative(Uri.parse(r).path, from: 'dartdoc/resources'))) { - expect( - resourceProvider.getFile(pathContext.join(output.path, resource)), - doesExist); - } - }); - }); - - group('for a package that causes duplicate files', () { - PackageGraph packageGraph; - var testPackageDuplicateDir = resourceProvider.getFolder( - pathContext.absolute( - pathContext.canonicalize('testing/test_package_duplicate'))); - - setUp(() async { - generator = await _initGeneratorForTest(); - packageGraph = await utils.bootBasicPackage( - testPackageDuplicateDir.path, - pubPackageMetaProvider, - PhysicalPackageConfigProvider()); - tempOutput = await resourceProvider.createSystemTemp('doc_test_temp'); - writer = DartdocFileWriter(tempOutput.path, resourceProvider); - }); - - tearDown(() { - if (tempOutput != null) { - tempOutput.delete(); - } - }); - - test('run generator and verify duplicate file error', () async { - await generator.generate(packageGraph, writer); - expect(generator, isNotNull); - expect(tempOutput, isNotNull); - var expectedPath = path.join('aDuplicate', 'aDuplicate-library.html'); - expect( - packageGraph.localPublicLibraries, - anyElement((l) => packageGraph.packageWarningCounter - .hasWarning(l, PackageWarning.duplicateFile, expectedPath))); - }, timeout: Timeout.factor(4)); - }); - }); -} - -const Matcher doesExist = _DoesExist(); - -class _DoesExist extends Matcher { - const _DoesExist(); - @override - bool matches(dynamic item, Map matchState) => (item as Resource).exists; - @override - Description describe(Description description) => description.add('exists'); - @override - Description describeMismatch(dynamic item, Description mismatchDescription, - Map matchState, bool verbose) { - if (item is! File && item is! Folder) { - return mismatchDescription - .addDescriptionOf(item) - .add('is not a file or directory'); - } else { - return mismatchDescription.add(' does not exist'); - } - } -} diff --git a/test/html_generator_test.dart b/test/html_generator_test.dart new file mode 100644 index 0000000000..54a77eee18 --- /dev/null +++ b/test/html_generator_test.dart @@ -0,0 +1,131 @@ +// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:analyzer/file_system/file_system.dart'; +import 'package:analyzer/file_system/memory_file_system.dart'; +import 'package:dartdoc/dartdoc.dart'; +import 'package:dartdoc/src/generator/generator_frontend.dart'; +import 'package:dartdoc/src/generator/html_generator.dart'; +import 'package:dartdoc/src/generator/html_resources.g.dart'; +import 'package:dartdoc/src/generator/templates.dart'; +import 'package:dartdoc/src/package_config_provider.dart'; +import 'package:dartdoc/src/package_meta.dart'; +import 'package:dartdoc/src/warnings.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +import 'src/utils.dart' as utils; + +void main() { + MemoryResourceProvider resourceProvider; + p.Context pathContext; + + PackageMetaProvider packageMetaProvider; + FakePackageConfigProvider packageConfigProvider; + + Templates templates; + GeneratorFrontEnd generator; + DartdocFileWriter writer; + + Folder projectRoot; + String projectPath; + + setUp(() async { + packageMetaProvider = utils.testPackageMetaProvider; + resourceProvider = packageMetaProvider.resourceProvider; + pathContext = resourceProvider.pathContext; + packageConfigProvider = utils + .getTestPackageConfigProvider(packageMetaProvider.defaultSdkDir.path); + + templates = await Templates.createDefault('html'); + generator = GeneratorFrontEnd(HtmlGeneratorBackend(null, templates)); + + projectRoot = utils.writePackage( + 'my_package', resourceProvider, packageConfigProvider); + projectPath = projectRoot.path; + var outputPath = projectRoot.getChildAssumingFolder('doc').path; + writer = DartdocFileWriter(outputPath, resourceProvider); + }); + + File getConvertedFile(String path) => + resourceProvider.getFile(resourceProvider.convertPath(path)); + + tearDown(() { + projectRoot = null; + projectPath = null; + clearPackageMetaCache(); + }); + + test('a null package has some assets', () async { + await generator.generate(null, writer); + var outputPath = projectRoot.getChildAssumingFolder('doc').path; + var output = resourceProvider + .getFolder(pathContext.join(outputPath, 'static-assets')); + expect(output, doesExist); + + for (var resource in resource_names.map((r) => + pathContext.relative(Uri.parse(r).path, from: 'dartdoc/resources'))) { + expect(resourceProvider.getFile(pathContext.join(output.path, resource)), + doesExist); + } + }); + + test('libraries with no duplicates are not warned about', () async { + getConvertedFile('$projectPath/lib/a.dart').writeAsStringSync('library a;'); + getConvertedFile('$projectPath/lib/b.dart').writeAsStringSync('library b;'); + var packageGraph = await utils.bootBasicPackage( + projectPath, packageMetaProvider, packageConfigProvider); + await generator.generate(packageGraph, writer); + + expect(packageGraph.packageWarningCounter.errorCount, 0); + }); + + test('libraries with duplicate names are warned about', () async { + getConvertedFile('$projectPath/lib/a.dart').writeAsStringSync('library a;'); + getConvertedFile('$projectPath/lib/b.dart').writeAsStringSync('library a;'); + var packageGraph = await utils.bootBasicPackage( + projectPath, packageMetaProvider, packageConfigProvider); + await generator.generate(packageGraph, writer); + + var expectedPath = pathContext.join('a', 'a-library.html'); + expect( + packageGraph.localPublicLibraries, + anyElement((l) => packageGraph.packageWarningCounter + .hasWarning(l, PackageWarning.duplicateFile, expectedPath))); + }); + + test('has HTML templates', () async { + expect(templates.indexTemplate, isNotNull); + expect(templates.libraryTemplate, isNotNull); + expect(templates.classTemplate, isNotNull); + expect(templates.functionTemplate, isNotNull); + expect(templates.constructorTemplate, isNotNull); + expect(templates.methodTemplate, isNotNull); + expect(templates.constantTemplate, isNotNull); + expect(templates.propertyTemplate, isNotNull); + expect(templates.topLevelConstantTemplate, isNotNull); + expect(templates.topLevelPropertyTemplate, isNotNull); + }); +} + +const Matcher doesExist = _DoesExist(); + +class _DoesExist extends Matcher { + const _DoesExist(); + @override + bool matches(Object item, Map matchState) => (item as Resource).exists; + @override + Description describe(Description description) => description.add('exists'); + @override + Description describeMismatch(Object item, Description mismatchDescription, + Map matchState, bool verbose) { + if (item is! File && item is! Folder) { + return mismatchDescription + .addDescriptionOf(item) + .add('is not a file or directory'); + } else { + return mismatchDescription.add(' does not exist'); + } + } +} diff --git a/test/library_test.dart b/test/library_test.dart index 4ecd63d2ed..f924283646 100644 --- a/test/library_test.dart +++ b/test/library_test.dart @@ -3,9 +3,6 @@ // BSD-style license that can be found in the LICENSE file. import 'package:analyzer/file_system/file_system.dart'; -import 'package:analyzer/file_system/memory_file_system.dart'; -import 'package:analyzer/src/test_utilities/mock_sdk.dart'; -import 'package:dartdoc/src/dartdoc_options.dart'; import 'package:dartdoc/src/model/model.dart'; import 'package:dartdoc/src/package_config_provider.dart'; import 'package:dartdoc/src/package_meta.dart'; @@ -14,34 +11,15 @@ import 'package:test/test.dart'; import 'src/utils.dart' as utils; void main() { - MemoryResourceProvider resourceProvider; - MockSdk mockSdk; Folder sdkFolder; PackageMetaProvider packageMetaProvider; FakePackageConfigProvider packageConfigProvider; setUp(() async { - resourceProvider = MemoryResourceProvider(); - mockSdk = MockSdk(resourceProvider: resourceProvider); - sdkFolder = utils.writeMockSdkFiles(mockSdk); - - packageMetaProvider = PackageMetaProvider( - PubPackageMeta.fromElement, - PubPackageMeta.fromFilename, - PubPackageMeta.fromDir, - resourceProvider, - sdkFolder, - defaultSdk: mockSdk, - ); - var optionSet = await DartdocOptionSet.fromOptionGenerators( - 'dartdoc', [createDartdocOptions], packageMetaProvider); - optionSet.parseArguments([]); - packageConfigProvider = FakePackageConfigProvider(); - // To build the package graph, we always ask package_config for a - // [PackageConfig] for the SDK directory. Put a dummy entry in. - packageConfigProvider.addPackageToConfigFor( - sdkFolder.path, 'analyzer', Uri.file('/sdk/pkg/analyzer/')); + packageMetaProvider = utils.testPackageMetaProvider; + sdkFolder = packageMetaProvider.defaultSdkDir; + packageConfigProvider = utils.getTestPackageConfigProvider(sdkFolder.path); }); test('libraries in SDK package have appropriate data', () async { diff --git a/test/package_test.dart b/test/package_test.dart index 04220b27e7..568b72fbb3 100644 --- a/test/package_test.dart +++ b/test/package_test.dart @@ -4,7 +4,6 @@ import 'package:analyzer/file_system/file_system.dart'; import 'package:analyzer/file_system/memory_file_system.dart'; -import 'package:analyzer/src/test_utilities/mock_sdk.dart'; import 'package:dartdoc/src/dartdoc_options.dart'; import 'package:dartdoc/src/model/documentable.dart'; import 'package:dartdoc/src/package_config_provider.dart'; @@ -19,7 +18,6 @@ void main() { PackageMetaProvider packageMetaProvider; FakePackageConfigProvider packageConfigProvider; DartdocOptionSet optionSet; - MockSdk mockSdk; Folder sdkFolder; Folder projectRoot; @@ -36,25 +34,13 @@ void main() { } setUp(() async { - resourceProvider = MemoryResourceProvider(); - mockSdk = MockSdk(resourceProvider: resourceProvider); - sdkFolder = utils.writeMockSdkFiles(mockSdk); - - packageMetaProvider = PackageMetaProvider( - PubPackageMeta.fromElement, - PubPackageMeta.fromFilename, - PubPackageMeta.fromDir, - resourceProvider, - sdkFolder, - defaultSdk: mockSdk, - ); + packageMetaProvider = utils.testPackageMetaProvider; + resourceProvider = packageMetaProvider.resourceProvider; + sdkFolder = packageMetaProvider.defaultSdkDir; + optionSet = await DartdocOptionSet.fromOptionGenerators( 'dartdoc', [createDartdocOptions], packageMetaProvider); - packageConfigProvider = FakePackageConfigProvider(); - // To build the package graph, we always ask package_config for a - // [PackageConfig] for the SDK directory. Put a dummy entry in. - packageConfigProvider.addPackageToConfigFor( - sdkFolder.path, 'analyzer', Uri.file('/sdk/pkg/analyzer/')); + packageConfigProvider = utils.getTestPackageConfigProvider(sdkFolder.path); }); tearDown(() { diff --git a/test/src/utils.dart b/test/src/utils.dart index 4c5663a642..3d911af85e 100644 --- a/test/src/utils.dart +++ b/test/src/utils.dart @@ -61,6 +61,32 @@ Future bootBasicPackage( .buildPackageGraph(); } +/// Returns a [FakePackageConfigProvider] with an entry for the SDK directory. +PackageConfigProvider getTestPackageConfigProvider(String sdkPath) { + var packageConfigProvider = FakePackageConfigProvider(); + // To build the package graph, we always ask package_config for a + // [PackageConfig] for the SDK directory. Put a dummy entry in. + packageConfigProvider.addPackageToConfigFor( + sdkPath, 'analyzer', Uri.file('/sdk/pkg/analyzer/')); + return packageConfigProvider; +} + +/// Returns a [PackageMetaProvider] using a [MemoryResourceProvider]. +PackageMetaProvider get testPackageMetaProvider { + var resourceProvider = MemoryResourceProvider(); + var mockSdk = MockSdk(resourceProvider: resourceProvider); + var sdkFolder = writeMockSdkFiles(mockSdk); + + return PackageMetaProvider( + PubPackageMeta.fromElement, + PubPackageMeta.fromFilename, + PubPackageMeta.fromDir, + resourceProvider, + sdkFolder, + defaultSdk: mockSdk, + ); +} + /// Writes [mockSdk] to disk at both its original path, and its canonicalized /// path (they may be different on Windows). /// diff --git a/testing/test_package_duplicate/lib/first.dart b/testing/test_package_duplicate/lib/first.dart deleted file mode 100644 index d369bdaa3c..0000000000 --- a/testing/test_package_duplicate/lib/first.dart +++ /dev/null @@ -1 +0,0 @@ -library aDuplicate; diff --git a/testing/test_package_duplicate/lib/second.dart b/testing/test_package_duplicate/lib/second.dart deleted file mode 100644 index d369bdaa3c..0000000000 --- a/testing/test_package_duplicate/lib/second.dart +++ /dev/null @@ -1 +0,0 @@ -library aDuplicate; diff --git a/testing/test_package_duplicate/pubspec.yaml b/testing/test_package_duplicate/pubspec.yaml deleted file mode 100644 index f0f27294b7..0000000000 --- a/testing/test_package_duplicate/pubspec.yaml +++ /dev/null @@ -1 +0,0 @@ -name: test_package_duplicate