Gen test runner in test task #315
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
aaronstgeorge-wf
approved these changes
Sep 26, 2019
lib/src/tasks/test/cli.dart
Outdated
| if (testFilePath.contains(_config.directory)) { | ||
| mapConfigToTestFiles.containsKey(_config) | ||
| ? mapConfigToTestFiles[_config].add(testFilePath) | ||
| : mapConfigToTestFiles[_config] = [testFilePath]; |
lib/src/tasks/test/cli.dart
Outdated
| final mapConfigToTestFiles = <TestRunnerConfig, | ||
| List<String> /* 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) { |
There was a problem hiding this comment.
To not shadow config
| List<String> /* 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) { |
There was a problem hiding this comment.
Would there be some issue if someone also passed the --release flag instead of or at the end of end of a list of files?
There was a problem hiding this comment.
.rest includes anything which wasn't specified as a top level option/flag, so we should be good here.
lib/src/tasks/test/cli.dart
Outdated
| // Empty all other unused generated runners | ||
| for (final _config in config.genTestRunner.configs) { | ||
| await genTestRunner(_config, filesToInclude: []); | ||
| } |
aaronstgeorge-wf
approved these changes
Sep 26, 2019
|
QA+1 - verified with a local dependency override in graph_app
|
evanweible-wf
approved these changes
Sep 30, 2019
|
QA +1
Merging into master. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
Incremental builds are slow for tests, since often a single library file can be transitively included in many tests, especially in the large repos. When there is a change in this file, that can cause very significant incremental build times.
This workaround adds a
--hack-fast-buildsflag, which re-writes the generated runners at run time in the test task. Use of this flag with a specific file to test will re-write the generated runners to include only this file. In practice, this dramatically improves incremental build times.Examples in graph_app:
When changing
lib/src/components/vertex_selector.dart(with an otherwise warm cache):--hack-fast-buildsflag, incremental rebuild time was12m 35s--hack-fast-buildsflag and specifying the test astest/unit/common/components/vertex_selector_test.dart:45sWhen changing
lib/src/sox_ui/src/flux/test_form/test_form_store.dart(with an otherwise warm cache):--hack-fast-buildsflag, incremental rebuild time was8m 9s--hack-fast-buildsflag and specifying the test astest/unit/sox_ui/flux/test_form/test_form_store_test.dart:36sTesting:
This modifies both the
testand thegen-test-runnertasks. Pick a repo, publink this branch, and run these steps:ddev testworks normally in dart 1ddev test path/to/file/works as it did in dart 1ddev testworks in dart 2ddev test path/to/fileworks in dart 2 and doesn’t re-write runnersddev test —hack-fast-builds path/to/file/works in dart 2 and re-writes runners, improving iterative build times, and re-writes runners back to original contents after tests run (Can usegit statusto verify):ddev gen-test-runner && ddev formatand verify there are not any code changes.