Optimize unit tests #5299
Optimize unit tests #5299
Conversation
vsavkin
added
the
action: review
vsavkin
force-pushed
the
make_broccoli_great_again
branch
2 times, most recently
from
b48c98a
to
437ceb1
Compare
| extensions: ['.js'], | ||
| onParseFile: function(data) { data.src = data.src.replace(/import \* as/g, "//import * as"); } | ||
| onParseFile: function(data) { data.src = data.src.replace(/\/\* circular \*\//g, "//"); } |
There was a problem hiding this comment.
There is no other way to do it :) The right way to do the check is to use cjs sources, not es6, and there we cannot rely on the import as trick.
There was a problem hiding this comment.
It is interesting that we have only one place in our code base where this is necessary. Maybe we can restructure our code to get rid of it.
There was a problem hiding this comment.
I spent a while on the other way to do it, and would love to show you :)
Will you be in the office today?
On Mon, Nov 16, 2015 at 9:20 AM Victor Savkin notifications@github.com
wrote:
In gulpfile.js
#5299 (comment):extensions: ['.js'],
- onParseFile: function(data) { data.src = data.src.replace(/import * as/g, "//import * as"); }
- onParseFile: function(data) { data.src = data.src.replace(//* circular *//g, "//"); }
There is no other way to do it :) The right way to do the check is to use
cjs sources, not es6, and there we cannot rely on the import as trick.—
Reply to this email directly or view it on GitHub
https://github.com/angular/angular/pull/5299/files#r44952906.
There was a problem hiding this comment.
I worked with Tobias on that circular dep. I think we should remove it by
making it a type-checker-only dependency. I'll show you.
Here was my work on this: #4454
On Mon, Nov 16, 2015 at 9:21 AM Alex Eagle alexeagle@google.com wrote:
I spent a while on the other way to do it, and would love to show you :)
Will you be in the office today?On Mon, Nov 16, 2015 at 9:20 AM Victor Savkin notifications@github.com
wrote:In gulpfile.js
#5299 (comment):extensions: ['.js'],
- onParseFile: function(data) { data.src = data.src.replace(/import * as/g, "//import * as"); }
- onParseFile: function(data) { data.src = data.src.replace(//* circular *//g, "//"); }
There is no other way to do it :) The right way to do the check is to use
cjs sources, not es6, and there we cannot rely on the import as trick.—
Reply to this email directly or view it on GitHub
https://github.com/angular/angular/pull/5299/files#r44952906.
|
discussed in person. lgtm |
IgorMinar
added
pr_state: LGTM
action: cleanup
vsavkin
force-pushed
the
make_broccoli_great_again
branch
from
70a031c
to
a6f63b9
Compare
vsavkin
removed
action: cleanup
Since editors and IDEs do typechecking and show errors in place, often there is no benefit to running type checking in our test pipeline. This PR allows you to disable type checking: gulp test.unit.js --noTypeChecks This commit also makes es6 generation optional. fix(build): removes unnecessary circular dependencies
vsavkin
force-pushed
the
make_broccoli_great_again
branch
from
a6f63b9
to
68422d3
Compare
|
Merging PR #5299 on behalf of @vsavkin to branch presubmit-vsavkin-pr-5299. |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Before:
initial 20s incremental: 5.5sBefore with noEmitOnError=false manually patched-in:
initial 18s incremental: 3.15sAfter:
initial 7.7s incremental: 0.5sIt is faster for the following two reasons:
Now, the time is split more-or-less equally between Funnel, MergeTrees, and TS.
This PR also fixed the circular dependencies check that was broken.