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

Add coverageExclude task #445

Closed

Conversation

rolandtritsch
Copy link
Member

@rolandtritsch rolandtritsch commented Jul 5, 2022

Hi @ckipp01,

this just shows/demos that and how the coverageExclude task works.

I know it is a "hack", but it is better than nothing and it works. I just tried it it on nmesos ...

image

I leave it with you to decide, if you want to consider this to include it in a potential 2.1.0 release.

Note: This also needs some minor changes to scala-scoverage-plugin

Note: This PR builds on top of this one.

@@ -8,6 +8,7 @@ object ScoverageKeys {
"controls whether code instrumentation is enabled or not"
)
lazy val coverageReport = taskKey[Unit]("run report generation")
lazy val coverageExclude = taskKey[Unit]("exclude/remove files and packages from coverage file (after the fact)")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the task that we add.

@@ -192,11 +194,45 @@ object ScoverageSbtPlugin extends AutoPlugin {
sjsClassifier getOrElse ""
}

private lazy val coverageExclude0 = Def.task {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the implementation of the task we add.

case None => log.warn("No coverage data, skipping reports")

case None =>
log.warn(s"No coverage data found in [$target]. Run >sbt coverage test< first.")
Copy link
Member Author

Choose a reason for hiding this comment

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

Making the warning more expressive.

case None =>
log.info("No subproject data to aggregate, skipping reports")
log.warn(s"No coverage data found to aggregate in [$target]. Run >sbt coverage test< first.")
Copy link
Member Author

Choose a reason for hiding this comment

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

Turning it into a warning (because the other log messages are warnings) and making it more expressive

@@ -386,10 +426,10 @@ object ScoverageSbtPlugin extends AutoPlugin {
sourceRoot: File
): Option[Coverage] = {

val dataDir = crossTarget / "/scoverage-data"
val dataDir = crossTarget / "scoverage-data"
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixing minor broken window.

@ckipp01
Copy link
Member

ckipp01 commented Jul 5, 2022

know it is a "hack", but it is better than nothing and it works.

Thanks for the pr @rolandtritsch, but as I mentioned in your request here I have no intention of maintaining a hack for this. Unless someone else would want to step up and maintain scoverage / sbt-scoverage I don't really have any intention of adding extra things except for bug fixes. I think this functionality should be added into dotc instead.

@rolandtritsch
Copy link
Member Author

rolandtritsch commented Jul 5, 2022

Hi @ckipp01. I want to help. Happy to take ownership to maintain sbt-scoverage. How can we make that happen?

And yes. As I said above I will also open a feature request on dotty.

@rolandtritsch
Copy link
Member Author

As a matter of fact the three repos I offer to own are ...

  • sbt-scoverage
  • sbt-scoverage-example
  • sbt-coveralls

Happy to help.

@ckipp01
Copy link
Member

ckipp01 commented Jul 19, 2022

Sorry for the delay @rolandtritsch. Working on getting you some access as I actually don't have rights to add you to the org.

@ckipp01
Copy link
Member

ckipp01 commented Jul 25, 2022

@rolandtritsch you should be good to go. I've got you added to the sbt team giving you access to the sbt repos. They should be set up to simply push a tag to release.

@rolandtritsch
Copy link
Member Author

Closing this for now.

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