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

[WIP] Enable CoffeeScript self coverage #4348

Closed
wants to merge 2 commits into from

Conversation

ryoqun
Copy link

@ryoqun ryoqun commented Oct 27, 2016

Hi, I've just hooked up coffee-coverage into coffee compiler itself to enable self-coverage as a proof of concept.

Indeed, this is very rough implementation (I've shamelessly just deleted some misbehaving test files and added hideous api just to work..). But it's mostly working fine. I've attached the result screenshots.

I think test coverage is a good thing for quality source code, so I want a good coverage support for CoffeeScript. Thereby, I'm trying to make it a first-class citizen by enabling it for the CoffeeScript itself (i.e., self-coverage akin to self-compile)!

If we can agree with this view, I'm happily invest some time for finishing it up! :)

benbria/coffee-coverage#81

@ryoqun ryoqun changed the title [wip] Coffee self coverage [wip] Enable CoffeeScript self coverage Oct 27, 2016
@ryoqun
Copy link
Author

ryoqun commented Oct 27, 2016

Greatly thanks to coffee-coverage and istanbul, the coverage report is really pretty. :)

screenshot_2016-10-27_13-58-58
screenshot_2016-10-27_14-01-24
screenshot_2016-10-27_14-04-20

@ryoqun
Copy link
Author

ryoqun commented Nov 4, 2016

(Sorry to bother you; I've picked up you guys from the pulse tab)

@lydell @GabrielRatener, if you have some time, could you take a look at this proposal?

I want to know if there is any possible for this to be merged, so I can continue this work toward to the finish. :)

@GeoffreyBooth
Copy link
Collaborator

I like the idea of this, though I think it should probably be submitted as a PR against 2. We changed the way tests run in 2 to handle async/await.

Can you please revert all the changes that would never get merged in? The deleted tests, the console.log/console.error statements, and so on? Personally I use SourceTree to review every file before I commit, and I only commit the changes I want to share, even within a file (like committing everything but a console.log, etc.). Not that you need to use SourceTree, but it would make your work much easier to review if you could be careful to commit only the changes you intend for potential approval.

What do you mean by “misbehaving” tests?

@ryoqun
Copy link
Author

ryoqun commented Dec 15, 2016

@GeoffreyBooth thanks for reply! :) I've been rather busy lately... It's not like I've given up this. :p

Recently another minor version released, Coffee got really love again. That's really delightful for me.

Still, if I find some time to spend, I plan to work on this in near future!

@GeoffreyBooth
Copy link
Collaborator

@ryoqun Do you plan on finishing this? As we’re approaching 2.0.0, now would be a good time 😄

@GeoffreyBooth GeoffreyBooth changed the title [wip] Enable CoffeeScript self coverage [WIP] Enable CoffeeScript self coverage Apr 22, 2017
@GeoffreyBooth
Copy link
Collaborator

coffee-coverage needs updating to support JSX: benbria/coffee-coverage#52; modules: benbria/coffee-coverage#82; and new CoffeeScript 2 features: benbria/coffee-coverage#88.

There was also a PR over there to make using coffee-coverage here possible: benbria/coffee-coverage#81; though it seems poorly implemented, and the code review notes were never addressed. It’s also unclear to me why it was necessary at all; adding require 'coffee-coverage/register-istanbul' to Cakefile seems to be enough.

Closing as this is a long way from being able to merge in. If someone can breathe new life into coffee-coverage and get it working with CoffeeScript 2, I would love to create a cake task for using it to check our test coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants