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

build(coverage): enable dev js code coverage #137

Closed
wants to merge 2 commits into from
Closed

build(coverage): enable dev js code coverage #137

wants to merge 2 commits into from

Conversation

joshwiens
Copy link
Contributor

Phase 1 of #128 - JavaScript coverage for local development

Adds the following npm devDependencies
- karma-coverage
- rimraf
Adds the following npm scripts both convienence items
- postinstall: installs typings & runs npm prune
- reinstall: cleans cache, generated content, node_modules & runs reinstall
Enables JavaScript coverage reporting with multiple outputs
- Reports code coverage at the commandline
- Generates an HTML coverage report
- Generates a coverage.json for future coverage history

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Mar 9, 2016
@@ -11,6 +11,8 @@
"build": "ng build",
"dartanalyzer": "cd dist/dart && pub install && cd ../.. && ts-node scripts/ci/dart_analyzer",
"demo-app": "cd src && ng serve",
"postinstall": "typings install --ambient && npm prune",
"reinstall": "npm cache clean && rimraf coverage dist typings && rimraf node_modules && npm install",
Copy link
Member

Choose a reason for hiding this comment

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

@hansl I think you had some opinions on postinstall scripts?

Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot have any postinstall/reinstall scripts as we still don't have a publishing story. So I would remove postinstall and reinstall until we know if we can use them. Or @jelbourn maybe it's okay for now since nobody can npm install material2 (yet).

Copy link
Contributor

Choose a reason for hiding this comment

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

And instead of running rimraf, just do a git clean -dfx.

Copy link
Contributor

Choose a reason for hiding this comment

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

After discussing, keep the postinstall/reinstall scripts. When we do the publishing process we'll revisit this decision.

Replace rimraf (and remove the dependency) by git clean -dfx. This will remove everything that's not part of the git repository, even cached files for everything. This is okay for a reinstall, which is the same as rebuilding everything anyway. Alternatively, we could do a git clean -dfx -e '.*' (single quotes important) to keep dotfiles around (such as .idea and .pub).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in c7c2799

  Adds the following npm devDependencies
    - karma-coverage
    - rimraf
  Adds the following npm scripts both convienence items
    - postinstall: installs typings & runs npm prune
    - reinstall: cleans cache, generated content node_modules & runs reinstall
  Enables JavaScript coverage reporting with multiple outputs
    - Reports code coverage at the commandline
    - Generates an HTML coverage report
    - Generates a coverage.json for future coverage history
preprocessors: {
'dist/**/!(*spec).js': ['coverage']
}
reporters: ['dots', 'coverage'],
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we force --reporters=dots on the command line in build-and-test.sh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

build-and-test.sh is only run in CI. The coverage currently is not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just something to keep in mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was something I was going to work out with you before I PR the typescript coverage, one of the two will have to change.

  Switches from rimraf to git to clean project
    - this still respects dot files
@joshwiens joshwiens closed this Mar 10, 2016
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants