From 56607eab86cdfb3f515a2cbdc2996e9b263c762d Mon Sep 17 00:00:00 2001 From: Corwin Sheahan Date: Tue, 17 Sep 2019 12:07:20 -0600 Subject: [PATCH 1/5] handle multiple runner instances --- lib/src/tasks/gen_test_runner/api.dart | 60 ++++++++++++----------- lib/src/tasks/gen_test_runner/config.dart | 8 ++- lib/src/tasks/test/cli.dart | 42 ++++++++++++++-- 3 files changed, 76 insertions(+), 34 deletions(-) diff --git a/lib/src/tasks/gen_test_runner/api.dart b/lib/src/tasks/gen_test_runner/api.dart index 42d0401b..020b9960 100644 --- a/lib/src/tasks/gen_test_runner/api.dart +++ b/lib/src/tasks/gen_test_runner/api.dart @@ -20,7 +20,7 @@ import 'dart:io'; import 'package:dart_dev/src/tasks/gen_test_runner/config.dart'; import 'package:dart_dev/src/tasks/task.dart'; -Future genTestRunner(TestRunnerConfig currentConfig) async { +Future genTestRunner(TestRunnerConfig currentConfig, {List filesToInclude}) async { var taskTitle = 'gen-test-runner'; var args = ['-d ${currentConfig.directory}', '-e ${currentConfig.env}']; currentConfig.genHtml ? args.add('--genHtml') : args.add('--no-genHtml'); @@ -28,13 +28,7 @@ Future genTestRunner(TestRunnerConfig currentConfig) async { GenTestRunnerTask task = new GenTestRunnerTask('$taskTitle ${args.join(' ')}'); - var currentDirectory = currentConfig.directory; - if (!currentDirectory.endsWith('/')) { - currentDirectory += '/'; - } - - File generatedRunner = - new File('${currentDirectory}${currentConfig.filename}.dart'); + File generatedRunner = new File(currentConfig.path); List existingLines; if (currentConfig.check) { @@ -46,29 +40,37 @@ Future genTestRunner(TestRunnerConfig currentConfig) async { } List runnerLines = []; - - Directory testDirectory = new Directory(currentDirectory); List testFiles = []; - final allFiles = testDirectory - .listSync(recursive: true, followLinks: false) - .where((entity) => entity is File) - .toList(); - allFiles.sort((left, right) => left.path.compareTo(right.path)); - allFiles - .where( - (entity) => !entity.path.endsWith('${currentConfig.filename}.dart')) - .forEach((entity) { - if (entity.path.endsWith('_test.dart')) { - testFiles.add(entity); - task.testFiles.add(entity.path); - } else if (entity.path.endsWith('.dart')) { - task.excludedFiles.add(entity.path); + + if (filesToInclude != null) { + for (final filePath in filesToInclude) { + print('adding $filePath'); + testFiles.add(new File(filePath)); } - }); + } else { + Directory testDirectory = new Directory(currentConfig.directory); + final allFiles = testDirectory + .listSync(recursive: true, followLinks: false) + .where((entity) => entity is File) + .toList(); + allFiles.sort((left, right) => left.path.compareTo(right.path)); + allFiles + .where( + (entity) => !entity.path.endsWith('${currentConfig.filename}.dart')) + .forEach((entity) { + if (entity.path.endsWith('_test.dart')) { + testFiles.add(entity); + task.testFiles.add(entity.path); + } else if (entity.path.endsWith('.dart')) { + task.excludedFiles.add(entity.path); + } + }); + } + print('testFiles: $testFiles'); if (currentConfig.genHtml && !currentConfig.check) { await testHtmlFileGenerator( - currentDirectory, currentConfig.filename, currentConfig.htmlHeaders); + currentConfig.directory, currentConfig.filename, currentConfig.htmlHeaders); } if (currentConfig.env == Environment.browser) { @@ -79,14 +81,14 @@ Future genTestRunner(TestRunnerConfig currentConfig) async { runnerLines.add('@TestOn(\'browser || vm\')'); } runnerLines.add( - 'library ${currentDirectory.replaceAll('/', '.')}${currentConfig.filename};'); + 'library ${currentConfig.directory.replaceAll('/', '.')}${currentConfig.filename};'); runnerLines.add(''); runnerLines.add('// Generated by `pub run dart_dev ${task.generateCommand}`'); runnerLines.add(''); testFiles.forEach((File file) { - var testPath = file.path.replaceFirst(currentDirectory, ''); + var testPath = file.path.replaceFirst(currentConfig.directory, ''); runnerLines.add( 'import \'${'./' + testPath}\' as ${testPath.replaceAll('/', '_').substring(0, testPath.length - 5)};'); }); @@ -109,7 +111,7 @@ Future genTestRunner(TestRunnerConfig currentConfig) async { } testFiles.forEach((File file) { - var testPath = file.path.replaceFirst(currentDirectory, ''); + var testPath = file.path.replaceFirst(currentConfig.directory, ''); runnerLines.add( ' ${testPath.replaceAll('/', '_').substring(0, testPath.length - 5)}.main();'); }); diff --git a/lib/src/tasks/gen_test_runner/config.dart b/lib/src/tasks/gen_test_runner/config.dart index ece808ab..6ebfc320 100644 --- a/lib/src/tasks/gen_test_runner/config.dart +++ b/lib/src/tasks/gen_test_runner/config.dart @@ -38,14 +38,18 @@ class TestRunnerConfig { bool genHtml = defaultGenHtml; List htmlHeaders = defaultHtmlHeaders; + String get path => '${directory + (!directory.endsWith('/') ? '/' : '')}$filename.dart'; + TestRunnerConfig( {List this.dartHeaders: defaultDartHeaders, List this.preTestCommands: defaultPreTestCommands, - String this.directory: defaultDirectory, + String directory: defaultDirectory, Environment this.env: defaultEnv, String this.filename: defaultFilename, bool this.genHtml: defaultGenHtml, - List this.htmlHeaders: defaultHtmlHeaders}); + List this.htmlHeaders: defaultHtmlHeaders}) { + this.directory = directory.endsWith('/') ? directory : directory + '/'; + } } class GenTestRunnerConfig extends TaskConfig { diff --git a/lib/src/tasks/test/cli.dart b/lib/src/tasks/test/cli.dart index ffec5257..62036d12 100644 --- a/lib/src/tasks/test/cli.dart +++ b/lib/src/tasks/test/cli.dart @@ -15,8 +15,12 @@ library dart_dev.src.tasks.test.cli; import 'dart:async'; +import 'dart:io'; import 'package:args/args.dart'; +import 'package:dart_dev/dart_dev.dart'; +import 'package:dart_dev/src/tasks/gen_test_runner/api.dart'; +import 'package:dart_dev/src/tasks/gen_test_runner/cli.dart'; import 'package:dart_dev/util.dart' show hasImmediateDependency, isPortBound, reporter, TaskProcess; @@ -98,8 +102,8 @@ class TestCli extends TaskCli { } final testArgs = []; - List tests = []; - List buildArgs = []; + final tests = []; + final buildArgs = []; if (!color) { testArgs.add('--no-color'); @@ -163,8 +167,33 @@ class TestCli extends TaskCli { // Build the list of tests to run. if (individualTestsSpecified) { + print('individual tests were specified'); // Individual tests explicitly passed in should override the test suites. - tests.addAll(parsedArgs.rest); + // Gen test runner + if (dartMajorVersion == 2) { + final mapConfigToTestFiles = /* tests to include in runner */>{}; + // Construct mapping from config to tests which should be ran in that config + for (final _config in config.genTestRunner.configs) { + for (final testFilePath in parsedArgs.rest) { + if (testFilePath.contains(_config.directory)) { + mapConfigToTestFiles.containsKey(_config) ? mapConfigToTestFiles[_config].add(testFilePath) : mapConfigToTestFiles[_config] = [testFilePath]; + } + } + } + + for (final _config in mapConfigToTestFiles.keys) { + config.genTestRunner.configs.remove(_config); + await genTestRunner(_config, filesToInclude: mapConfigToTestFiles[_config]); + tests.add(_config.path); + } + + // Empty all other unused generated runners + for (final _config in config.genTestRunner.configs) { + await genTestRunner(_config, filesToInclude: []); + } + } else { + tests.addAll(parsedArgs.rest); + } } else { // Unit and/or integration suites should only run if individual tests // were not specified. @@ -285,6 +314,13 @@ A pub serve instance will not be started.'''); await runAll(config.test.after = config.test.afterFunctionalTests); } + + // Regenerate all runners: + for (final _config in config.genTestRunner.configs) { + print('regenerating runners'); + await genTestRunner(_config); + } + return task.successful ? new CliResult.success(task.testSummary) : new CliResult.fail(task.testSummary); From c9fa417ecd3c5e7db9340860931acc7a8167334b Mon Sep 17 00:00:00 2001 From: Corwin Sheahan Date: Thu, 19 Sep 2019 12:01:47 -0600 Subject: [PATCH 2/5] cleanup --- lib/src/tasks/gen_test_runner/api.dart | 2 -- lib/src/tasks/test/cli.dart | 2 -- 2 files changed, 4 deletions(-) diff --git a/lib/src/tasks/gen_test_runner/api.dart b/lib/src/tasks/gen_test_runner/api.dart index 020b9960..f54a2cb6 100644 --- a/lib/src/tasks/gen_test_runner/api.dart +++ b/lib/src/tasks/gen_test_runner/api.dart @@ -44,7 +44,6 @@ Future genTestRunner(TestRunnerConfig currentConfig, {List genTestRunner(TestRunnerConfig currentConfig, {List Date: Tue, 24 Sep 2019 16:37:34 -0600 Subject: [PATCH 3/5] Hide generated runner re-writes behind a flag --- lib/src/tasks/gen_test_runner/api.dart | 15 +++--- lib/src/tasks/gen_test_runner/config.dart | 10 ++-- lib/src/tasks/test/cli.dart | 62 +++++++++++++++-------- 3 files changed, 55 insertions(+), 32 deletions(-) diff --git a/lib/src/tasks/gen_test_runner/api.dart b/lib/src/tasks/gen_test_runner/api.dart index f54a2cb6..cb0df4c1 100644 --- a/lib/src/tasks/gen_test_runner/api.dart +++ b/lib/src/tasks/gen_test_runner/api.dart @@ -20,7 +20,8 @@ import 'dart:io'; import 'package:dart_dev/src/tasks/gen_test_runner/config.dart'; import 'package:dart_dev/src/tasks/task.dart'; -Future genTestRunner(TestRunnerConfig currentConfig, {List filesToInclude}) async { +Future genTestRunner(TestRunnerConfig currentConfig, + {List filesToInclude}) async { var taskTitle = 'gen-test-runner'; var args = ['-d ${currentConfig.directory}', '-e ${currentConfig.env}']; currentConfig.genHtml ? args.add('--genHtml') : args.add('--no-genHtml'); @@ -67,8 +68,8 @@ Future genTestRunner(TestRunnerConfig currentConfig, {List genTestRunner(TestRunnerConfig currentConfig, {List genTestRunner(TestRunnerConfig currentConfig, {List dartHeaders = defaultDartHeaders; List preTestCommands = defaultPreTestCommands; String directory = defaultDirectory; + String get normalizedDirectory => + directory + (!directory.endsWith('/') ? '/' : ''); Environment env = defaultEnv; String filename = defaultFilename; bool genHtml = defaultGenHtml; List htmlHeaders = defaultHtmlHeaders; - String get path => '${directory + (!directory.endsWith('/') ? '/' : '')}$filename.dart'; + String get path => '$normalizedDirectory$filename.dart'; TestRunnerConfig( {List this.dartHeaders: defaultDartHeaders, List this.preTestCommands: defaultPreTestCommands, - String directory: defaultDirectory, + String this.directory: defaultDirectory, Environment this.env: defaultEnv, String this.filename: defaultFilename, bool this.genHtml: defaultGenHtml, - List this.htmlHeaders: defaultHtmlHeaders}) { - this.directory = directory.endsWith('/') ? directory : directory + '/'; - } + List this.htmlHeaders: defaultHtmlHeaders}); } class GenTestRunnerConfig extends TaskConfig { diff --git a/lib/src/tasks/test/cli.dart b/lib/src/tasks/test/cli.dart index 721a9de3..c45fe138 100644 --- a/lib/src/tasks/test/cli.dart +++ b/lib/src/tasks/test/cli.dart @@ -21,6 +21,7 @@ import 'package:args/args.dart'; import 'package:dart_dev/dart_dev.dart'; import 'package:dart_dev/src/tasks/gen_test_runner/api.dart'; import 'package:dart_dev/src/tasks/gen_test_runner/cli.dart'; +import 'package:dart_dev/src/tasks/gen_test_runner/config.dart'; import 'package:dart_dev/util.dart' show hasImmediateDependency, isPortBound, reporter, TaskProcess; @@ -43,6 +44,8 @@ class TestCli extends TaskCli { ..addFlag('functional', defaultsTo: defaultFunctional, help: 'Includes the functional test suite.') + ..addFlag('delete-conflicting-outputs', + help: 'Deletes conflicting outputs during the build', negatable: false) ..addFlag('disable-serve-std-out', defaultsTo: defaultDisableServeStdOut, help: 'Disables standard output for pub serve task.') @@ -50,6 +53,12 @@ class TestCli extends TaskCli { abbr: 'j', defaultsTo: '$defaultConcurrency', help: 'The number of concurrent test suites run.') + ..addFlag(_hackFastBuilds, + help: + 'Improves iterative build times by re-writing the generated test runners at runtime.\n' + 'Use this flag and specify the test file you want to run.\nThis flag ' + 'is a no-op if no test files are specified.', + defaultsTo: false) ..addFlag('pub-serve', negatable: true, defaultsTo: defaultPubServe, @@ -59,8 +68,6 @@ class TestCli extends TaskCli { 'Implies --concurrency=1 and --timeout=none.\n' 'Currently only supported for browser tests.', negatable: false) - ..addFlag('delete-conflicting-outputs', - help: 'Deletes conflicting outputs during the build', negatable: false) ..addFlag('release', abbr: 'r', negatable: true, @@ -170,25 +177,35 @@ class TestCli extends TaskCli { // Individual tests explicitly passed in should override the test suites. // Gen test runner if (dartMajorVersion == 2) { - final mapConfigToTestFiles = /* tests to include in runner */>{}; - // Construct mapping from config to tests which should be ran in that config - for (final _config in config.genTestRunner.configs) { - for (final testFilePath in parsedArgs.rest) { - if (testFilePath.contains(_config.directory)) { - mapConfigToTestFiles.containsKey(_config) ? mapConfigToTestFiles[_config].add(testFilePath) : mapConfigToTestFiles[_config] = [testFilePath]; + if (parsedArgs[_hackFastBuilds]) { + reporter.warning( + 'You\'re using `${_hackFastBuilds}`. This will re-write the generated test runners in your repo.\n' + 'The test task will attempt to restore your generated runners after completion, but you may ' + 'have to re-run `pub run dart_dev gen-test-runner` if your runners have changed.'); + final mapConfigToTestFiles = /* tests to include in runner */ >{}; + // Construct mapping from config to tests which should be ran in that config + for (final _config in config.genTestRunner.configs) { + for (final testFilePath in parsedArgs.rest) { + if (testFilePath.contains(_config.directory)) { + mapConfigToTestFiles.containsKey(_config) + ? mapConfigToTestFiles[_config].add(testFilePath) + : mapConfigToTestFiles[_config] = [testFilePath]; + } } } - } - for (final _config in mapConfigToTestFiles.keys) { - config.genTestRunner.configs.remove(_config); - await genTestRunner(_config, filesToInclude: mapConfigToTestFiles[_config]); - tests.add(_config.path); - } + for (final _config in mapConfigToTestFiles.keys) { + config.genTestRunner.configs.remove(_config); + await genTestRunner(_config, + filesToInclude: mapConfigToTestFiles[_config]); + tests.add(_config.path); + } - // Empty all other unused generated runners - for (final _config in config.genTestRunner.configs) { - await genTestRunner(_config, filesToInclude: []); + // Empty all other unused generated runners + for (final _config in config.genTestRunner.configs) { + await genTestRunner(_config, filesToInclude: []); + } } } else { tests.addAll(parsedArgs.rest); @@ -313,14 +330,17 @@ A pub serve instance will not be started.'''); await runAll(config.test.after = config.test.afterFunctionalTests); } - - // Regenerate all runners: - for (final _config in config.genTestRunner.configs) { - await genTestRunner(_config); + if (parsedArgs[_hackFastBuilds]) { + // Regenerate all runners: + for (final _config in config.genTestRunner.configs) { + await genTestRunner(_config); + } } return task.successful ? new CliResult.success(task.testSummary) : new CliResult.fail(task.testSummary); } + + static String _hackFastBuilds = 'hack-fast-builds'; } From 04380c6443afb51ab91504baa607ad3741b578ef Mon Sep 17 00:00:00 2001 From: Corwin Sheahan Date: Wed, 25 Sep 2019 13:43:59 -0600 Subject: [PATCH 4/5] Retain normal test specified behavior if flag isn't set --- lib/src/tasks/test/cli.dart | 51 +++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/lib/src/tasks/test/cli.dart b/lib/src/tasks/test/cli.dart index c45fe138..02a5ff4f 100644 --- a/lib/src/tasks/test/cli.dart +++ b/lib/src/tasks/test/cli.dart @@ -175,38 +175,35 @@ class TestCli extends TaskCli { // Build the list of tests to run. if (individualTestsSpecified) { // Individual tests explicitly passed in should override the test suites. - // Gen test runner - if (dartMajorVersion == 2) { - if (parsedArgs[_hackFastBuilds]) { - reporter.warning( - 'You\'re using `${_hackFastBuilds}`. This will re-write the generated test runners in your repo.\n' - 'The test task will attempt to restore your generated runners after completion, but you may ' - 'have to re-run `pub run dart_dev gen-test-runner` if your runners have changed.'); - final mapConfigToTestFiles = /* tests to include in runner */ >{}; - // Construct mapping from config to tests which should be ran in that config - for (final _config in config.genTestRunner.configs) { - for (final testFilePath in parsedArgs.rest) { - if (testFilePath.contains(_config.directory)) { - mapConfigToTestFiles.containsKey(_config) - ? mapConfigToTestFiles[_config].add(testFilePath) - : mapConfigToTestFiles[_config] = [testFilePath]; - } + if (dartMajorVersion == 2 && parsedArgs[_hackFastBuilds]) { + reporter.warning( + 'You\'re using `${_hackFastBuilds}`. This will re-write the generated test runners in your repo.\n' + 'The test task will attempt to restore your generated runners after completion, but you may ' + 'have to re-run `pub run dart_dev gen-test-runner` if your runners have changed.'); + final mapConfigToTestFiles = /* tests to include in runner */>{}; + // Construct mapping from config to tests which should be ran in that config + for (final _config in config.genTestRunner.configs) { + for (final testFilePath in parsedArgs.rest) { + if (testFilePath.contains(_config.directory)) { + mapConfigToTestFiles.containsKey(_config) + ? mapConfigToTestFiles[_config].add(testFilePath) + : mapConfigToTestFiles[_config] = [testFilePath]; } } + } - for (final _config in mapConfigToTestFiles.keys) { - config.genTestRunner.configs.remove(_config); - await genTestRunner(_config, - filesToInclude: mapConfigToTestFiles[_config]); - tests.add(_config.path); - } + for (final _config in mapConfigToTestFiles.keys) { + config.genTestRunner.configs.remove(_config); + await genTestRunner(_config, + filesToInclude: mapConfigToTestFiles[_config]); + tests.add(_config.path); + } - // Empty all other unused generated runners - for (final _config in config.genTestRunner.configs) { - await genTestRunner(_config, filesToInclude: []); + // Empty all other unused generated runners + for (final _config in config.genTestRunner.configs) { + await genTestRunner(_config, filesToInclude: []); } - } } else { tests.addAll(parsedArgs.rest); } From f20908ba281b05666b27a935cb758112431c8f70 Mon Sep 17 00:00:00 2001 From: Corwin Sheahan Date: Thu, 26 Sep 2019 13:50:40 -0600 Subject: [PATCH 5/5] write runners with their original content --- lib/src/tasks/test/cli.dart | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/lib/src/tasks/test/cli.dart b/lib/src/tasks/test/cli.dart index 02a5ff4f..8f56c1ae 100644 --- a/lib/src/tasks/test/cli.dart +++ b/lib/src/tasks/test/cli.dart @@ -172,38 +172,41 @@ class TestCli extends TaskCli { 'files/directories'); } + final mapRunnerToContents = {}; // Build the list of tests to run. if (individualTestsSpecified) { // Individual tests explicitly passed in should override the test suites. if (dartMajorVersion == 2 && parsedArgs[_hackFastBuilds]) { reporter.warning( - 'You\'re using `${_hackFastBuilds}`. This will re-write the generated test runners in your repo.\n' + 'WARNING: You\'re using `${_hackFastBuilds}`. This will re-write the generated test runners in your repo.\n' 'The test task will attempt to restore your generated runners after completion, but you may ' - 'have to re-run `pub run dart_dev gen-test-runner` if your runners have changed.'); + 'have to re-run `pub run dart_dev gen-test-runner` and `pub run dart_dev format` if your runners have changed.\n\n'); final mapConfigToTestFiles = /* tests to include in runner */>{}; + Set /* tests to include in runner */>{}; // Construct mapping from config to tests which should be ran in that config - for (final _config in config.genTestRunner.configs) { + final copyOfConfigs = new List.from(config.genTestRunner.configs); + for (final _config in copyOfConfigs) { for (final testFilePath in parsedArgs.rest) { if (testFilePath.contains(_config.directory)) { - mapConfigToTestFiles.containsKey(_config) - ? mapConfigToTestFiles[_config].add(testFilePath) - : mapConfigToTestFiles[_config] = [testFilePath]; + mapConfigToTestFiles.putIfAbsent(_config, () => new Set.from([testFilePath])); + mapConfigToTestFiles[_config].add(testFilePath); } } } for (final _config in mapConfigToTestFiles.keys) { - config.genTestRunner.configs.remove(_config); + copyOfConfigs.remove(_config); + final runnerFile = new File(_config.path); + mapRunnerToContents.putIfAbsent(runnerFile, () => runnerFile.readAsStringSync()); await genTestRunner(_config, - filesToInclude: mapConfigToTestFiles[_config]); + filesToInclude: mapConfigToTestFiles[_config].toList()); tests.add(_config.path); } // Empty all other unused generated runners - for (final _config in config.genTestRunner.configs) { + for (final _config in copyOfConfigs) { await genTestRunner(_config, filesToInclude: []); - } + } } else { tests.addAll(parsedArgs.rest); } @@ -329,9 +332,9 @@ A pub serve instance will not be started.'''); if (parsedArgs[_hackFastBuilds]) { // Regenerate all runners: - for (final _config in config.genTestRunner.configs) { - await genTestRunner(_config); - } + mapRunnerToContents.forEach((file, originalContents) { + file.writeAsStringSync(originalContents); + }); } return task.successful