-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
enabled coverage reports for browsers Solves #8632 #9308
Changes from 3 commits
a4dcd10
ab239ce
93eb09b
e662d41
697d5d6
b62118b
23a8f3b
88c6de3
3c6b2e8
71ce53f
bb41b2e
a12ea4b
fdf0cdb
05ac26e
796b35e
2d989b2
2558dec
a502102
1180f3f
3fb4982
e14841e
bfb48d4
884478d
2e3db84
4d803fe
2b19337
647ba0e
beb7bd9
12eda40
6db3cee
88871b1
da27bfd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,5 @@ node_modules/ | |
examples/node/svgdump/ | ||
examples/node/pdf2png/*.png | ||
package-lock.json | ||
.idea | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,3 +9,6 @@ install: | |
- npm install -g gulp-cli | ||
- npm install | ||
- npm update | ||
|
||
after_success: | ||
- npm run coveralls |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,8 @@ var zip = require('gulp-zip'); | |
var webpack2 = require('webpack'); | ||
var webpackStream = require('webpack-stream'); | ||
var vinyl = require('vinyl-fs'); | ||
var istanbul = require('gulp-istanbul'); | ||
var inject = require('gulp-inject'); | ||
|
||
var BUILD_DIR = 'build/'; | ||
var L10N_DIR = 'l10n/'; | ||
|
@@ -84,6 +86,11 @@ var DEFINES = { | |
SKIP_BABEL: false, | ||
}; | ||
|
||
var ISTANBUL_PATHS = { | ||
'javascript': ['coverage/**/*.js'], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the single quotes around |
||
tests: ['test/**/*.js'], | ||
}; | ||
|
||
function safeSpawnSync(command, parameters, options) { | ||
// Execute all commands in a shell. | ||
options = options || {}; | ||
|
@@ -1168,6 +1175,29 @@ gulp.task('lint', function (done) { | |
}); | ||
}); | ||
|
||
gulp.task('instrument', function () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add |
||
return gulp.src(['src/**/*.js']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you using |
||
// Covering files | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the purpose of this comment. Could you either remove it or rephrase it? And if you choose to keep the comment, please add space before the line so that it is aligned consistently with the following lines. |
||
.pipe(istanbul({ coverageVariable: '__coverage__', | ||
})) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This formatting looks a bit weird. Can you merge it with the previous line? |
||
// instrumented files will go here | ||
.pipe(gulp.dest('coverage/')); | ||
}); | ||
|
||
gulp.task('inject', ['instrument'], function (cb) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename |
||
return gulp.src('index.html') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole task is not doing anything because there is no file called "index.html". |
||
.pipe(inject( | ||
gulp.src(ISTANBUL_PATHS.javascript, { read: false, }), { | ||
relative: true, | ||
})) | ||
.pipe(inject( | ||
gulp.src(ISTANBUL_PATHS.tests, { read: false, }), { | ||
relative: true, | ||
starttag: '<!-- inject:tests:js -->', | ||
})) | ||
.pipe(gulp.dest('.')); | ||
}); | ||
|
||
gulp.task('server', function (done) { | ||
console.log(); | ||
console.log('### Starting local server'); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
"babel-plugin-transform-es2015-modules-commonjs": "^6.26.0", | ||
"babel-preset-env": "^1.6.0", | ||
"core-js": "^2.5.1", | ||
"coveralls": "^3.0.0", | ||
"escodegen": "^1.9.0", | ||
"eslint": "^4.10.0", | ||
"eslint-plugin-mozilla": "^0.4.9", | ||
|
@@ -18,6 +19,7 @@ | |
"gulp-transform": "^3.0.5", | ||
"gulp-util": "^3.0.8", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where are you using this dependency? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll remove this ok |
||
"gulp-zip": "^4.0.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you downgrade the gulp-zip dependency? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll change it back to the same |
||
"istanbul": "^1.0.0-alpha.2", | ||
"jasmine": "^2.8.0", | ||
"jasmine-core": "^2.8.0", | ||
"jsdoc": "^3.5.5", | ||
|
@@ -38,11 +40,17 @@ | |
"yargs": "^9.0.1" | ||
}, | ||
"scripts": { | ||
"test": "gulp lint unittestcli externaltest" | ||
"test": "gulp lint unittestcli externaltest", | ||
"cover": "istanbul cover --include-all-sources jasmine JASMINE_CONFIG_PATH=test/unit/clitests.json", | ||
"coveralls": "npm run cover && cat ./coverage/lcov.info | coveralls" | ||
}, | ||
"repository": { | ||
"type": "git", | ||
"url": "git://github.com/mozilla/pdf.js.git" | ||
}, | ||
"license": "Apache-2.0" | ||
"license": "Apache-2.0", | ||
"dependencies": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move the dependencies below to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Rob--W Thanx for all these reviews, I'll make all the changes accordingly. |
||
"gulp-inject": "^4.3.0", | ||
"gulp-istanbul": "^1.1.2" | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. None of these dependencies are used - remove them. In the future, if you add new dependencies, store them in the |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be here, please remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure will do that in my next commit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is still here (don't forget to remove the new trailing line too).