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

RFC: Add --files to allow running specific test files. #3091

Merged
merged 2 commits into from
May 4, 2016

Conversation

mkhatib
Copy link
Contributor

@mkhatib mkhatib commented May 4, 2016

Following are test runs for test-amp-iframe.js with the following command:

Test Single Run Subsequent Watched Runs
gulp test (describe.only - current flow)
gulp test
59s ~8s
gulp test (--files - build all extensions)
gulp test --files=extensions/amp-iframe/0.1/test/*.js
36s ~6s
gulp test (--files build only extensions being tested - this change)
gulp test --files=extensions/amp-iframe/0.1/test/*.js
18s ~4s

Even with no speed wins this also allows us to run tests that are in multiple files or multiple describe blocks in the same file - currently not possible, one would have to keep doing .only on multiple describe blocks. Speed wins also gonna keep accumulate as we add more and more extensions.

In general this hopefully could be very useful during development where one is just incrementally adding tests and testing functionality in general.

I general this can even be faster with more optimization to what needs to be built or compiled. For example, as far as I can tell one wouldn't need to run compile in build step for running the unit tests, though not entirely sure (for example, the test-amp-iframe runs in 12s instead of 18s if compile is dropped). One can also probably skip building examples during running these kind of tests (though the saves from examples might be insignificant - ~1s - another ~1s for dropping buildAlp when not needed).

I think more importantly, this get rid of a lot of .only usage and forgetting it in source code to only realize the test failed after travis has ran presubmit - some usage still probably useful to run a single describe or it blocks.

cc/ @cramforce @dvoytenko @jridgewell @sriramkrish85 @erwinmombay

const files = [];
const passedPaths = Array.isArray(argv.files) ? argv.files : [argv.files];
config.commonTestPaths.forEach(file => files.push(file));
passedPaths.forEach(file => files.push(file));
Copy link
Member

@erwinmombay erwinmombay May 4, 2016

Choose a reason for hiding this comment

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

just do:

files = [].concat(config.commonTestPaths, argv.files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

woah thanks 👍 Done

@erwinmombay
Copy link
Member

LGTM

@mkhatib mkhatib merged commit c0de4bf into ampproject:master May 4, 2016
@mkhatib mkhatib deleted the tests-specifc-files branch May 4, 2016 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants