Skip to content
This repository has been archived by the owner on Aug 7, 2020. It is now read-only.

First pass at generating cobertura-style output using string templates #107

Merged
merged 1 commit into from
Sep 27, 2013

Conversation

sieverssj
Copy link
Contributor

These changes are by no means complete, but I wanted to get some feedback on them before continuing development. I know the ultimate goal for Saga is to get away from string templates for reports, however, I think using string templates to generate the cobertura report for now is much simpler.

One key bit of work that still remains is to group the files by package/directory so the results are easier to look at. Currently, all files will end up in a default package.

One other change I had to make but didn't like was adding a "sourceDirs" property to the configuration in for the maven integration. I couldn't find any other good way to populate the source directories in the cobertura report, especially when Saga was running with Jasmine, where all sources appear to come from http://localhost:1234

I would also like to be able to generate complexity statistics, method coverage, and branch coverage in the future, but that's going to rely on other work to get those stats from the coverage generators.

If this seems like a good start, I can continue the work to group the coverage by package/directory.

Adding a "sourceDir" property in the mojo to allow us to generate Cobertura reports with proper source pointers.

Adding the ability to define multiple source directories to be listed in Cobertura report output.
@sieverssj
Copy link
Contributor Author

I should have clarified the source directory changes a bit. In the pom, you can specify a configuration for the source directories, but I think for most projects something like this would suffice:

<sourceDirs>
  <sourceDir>${project.basedir}</sourceDir>
</sourceDirs>

I'll have to investigate if there is a way to make that configuration the default in the mojo instead of relying on consumers to update their poms.

@timurstrekalov
Copy link
Owner

Looks very cool, nice and clean 👍

Do you want to continue working on this PR (if you still have something) or should I merge this already? Do you think you could add some tests for this somehow? I know the project lacks in this department, but it might be a good idea to start making it better, if possible.

@sieverssj
Copy link
Contributor Author

I'll continue working on this if you want to hold off on the merge. I'd like to at least get the package level stats together before the merge.

I'll take a look at unit tests and see what can be done.

@timurstrekalov
Copy link
Owner

Awesome!

@sieverssj
Copy link
Contributor Author

@timurstrekalov 8 days ago I was much more optimistic about having time to work on the changes I outlined above. :) I may take you up on the offer to merge this pull request and I'll work on the changes as another pull request. Is that doable?

Everything in the pull request will work, it just isn't the ideal solution at this stage.

@timurstrekalov
Copy link
Owner

@sieverssj I think that's totally fine. I'll merge this one now then.

Thanks a ton!

timurstrekalov added a commit that referenced this pull request Sep 27, 2013
First pass at generating cobertura-style output using string templates
@timurstrekalov timurstrekalov merged commit 80cfcff into timurstrekalov:master Sep 27, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants