Coverage Broken: why does nobody care?

In an attempt to be solution oriented, I would agree with @odersky that it makes sense to give tracking of this issue a proper home.

I see the original poster has attempted a fix or linked to a fix, is there anyway the compiler teams can help the community get up to speed with what needs to be known to take ownership of this area of tooling?

I for one have watched the compiler videos and even signed up for a spree (but scheduling conflicts caused me to be unable to participate after all), but don’t feel knowledgeable enough to even know where to start on this, and I applaud the OP and others for taking this up. I felt encouraged to see someone else decided to take up this effort and would love to help as well.

5 Likes

You are being unfair to people who are actually doing the work here. From my perspective, someone came in to raise a problem, I took a look at the linked PR, suggested a way forward, and made a note to myself to follow up on it. I can’t control everyone’s reaction in a public forum, and I can’t immediately fix every issue.

There are ways for companies to influence the work being done on Scala, for example by sponsoring the Scala Center, but I don’t think that threats of abandoning Scala will achieve anything positive.

The compiler sprees are great for this, but as I mentioned earlier in this thread, anyone is welcome to come on Discord to chat about an issue and we can organize impromptu pair-programming sessions.

10 Likes

Thanks for highlighting that. I haven’t looked at using the interactions in that channel that way, and I’ll keep that in mind going forward.

1 Like

I’m not being “unfair” to anyone - I am stating facts. Attacking people who point out that something as basic as test coverage verification hasn’t worked for a year will have a negative effect on the success of Scala, and THAT is what people should be concerned about, primarily, certainly more than feelings.

(The childishness, infighting, and politicization of the Scala ecosystem is a related subject that also works against the success of the language.)

That’s about the last of what I have to say on this particular subject.

3 Likes

No one is trying to threaten. It’s a very real problem that Scala is losing to the competition that many of us are very scared about and bothered by.

I get that Scala is seriously under-resourced. But IMO this is more about the attitude that is often projected. And I’m not talking about you. But all the same facts on the ground could be the same, and there could be someone who projects leadership, talks about how and when this will get done, tries to make connections to speed it up, and sets expectations that make people feel supported, not simply be defensive about why it hasn’t been done yet.

Some of this is just about talking about the full half of the cup instead of the empty half.

Also as alluded, it’s possible there are companies that would love to put money into it but don’t have where to put the money to make it happen. Someone who wants Scala to succeed could take the initiative of trying to make that happen.

A given company, left to their own devices, may not prefer that approach, since they can equally stay on scala 2 and/or invest in other languages. But if someone reaches out to them maybe they could be persuaded.

Every time people that represent Scala reply on the forum, they should see it as a PR situation, because unfortunately that’s what it is. If people see it as a ragtag band of stressed out solo developers they will go elsewhere. That doesn’t mean you should take on too much and get burnt out. It’s about the messaging.

(Also, I hope that tooling is being prioritized over cool new improvements, not for myself but so that Scala can be more popular in the US.)

9 Likes

Hello! I am just scala language user. I have nothing with the compiler team or scala organization. But I noticed that the compiler team is unsure about who is responsible for this task. Could you please put someone on this task that is capable of doing this? And fill in the cracks in the process. Something Unassigned to anyone must be assigned to someone that will know what to do next.

I believe that it’s important issues like this to be taken seriously.

@odersky group, you created a beautiful language, and I think it would be great if we could put it to good use for everybody.

And industry adoption is really REQUIRED for success. Now that Scala 3.3.0 is out, what are the priorities? Is there a way for the community to vote on which priorities should be addressed first? This coverage must be voted up! Is there a mechanism to prioritize tasks like this? JetBrains has YouTrack tool, which looks cool. Does scala compiler team / tooling has one? If not make it.Maybe this voices concerns “why nobody cares”, maybe because there is not mechanism to hear the voice of your userbase, except this form. Listening frustrations here is not a pleasant experience. If scala lang org needs funding let us know(blog post?) and we can donate, you will need to make your compiler team bigger, maybe funding can go in this direction and we can yearly donate what we can. If scala is bringing success to the people why not giving something back. I really am grateful to all open source developer on this project. You are doing fine job with the language, but some work with greese needs to be done otherwise your open source contributions will not see much of a future adoption. And I believe that you(scala organisation) can do it somehow.

4 Likes

One engineer from VirtusLab already started working on coverage bugs.

It is unfortunate that this area fell through the cracks in terms of responsibility. I’ll take care that things like this will not happen in the future.

21 Likes

These companies might also be interested in helping improve the Scala 2 coverage implementation then, consider:

import java.nio.file._

object Util {
  def createFile(fileName: String): Path = {
    Files.createFile(
      return Paths.get(fileName)
    )
  }
}

createFile is never called because of the stray return, but in Scala 2 we get:


Whereas in Scala 3 we get:

So neither is right, but Scala 2 is over-counting the coverage percentage whereas Scala 3 is under-counting. The latter is more noticeable, but the former seems more problematic to me since it gives you a false sense of confidence. I don’t think either issue is due to carelessness, this stuff is just hard to get right.

8 Likes

I would daresay that it’s better to over-count this. It is not the job of code-coverage to find incorrect runtime code-paths (or at least incorrect ones according to what our human definitions are) because this requires understanding what a user’s intentions are, which is the job of the actual unit-test’s assertion clauses. They’re the ones that should catch this problem, not the coverage tool.

The conceptual model that most users have about code-coverage is based on an abstract notion of what represents a “line of code” and what “lines of code” get “hit.” That means that if the actual line representing the argument passed to Paths.get is invoked in some kind of way, it should be counted as “hit,” even if it technically represents a behavior that does not cause the execution of the surrounding function.

When code-coverage attempts to be more clever than this “line being hit” model, it inherently introduces unexpected behavior that confuses and irritates users. Coverage checkers should be as simple and naive as possible.

2 Likes

Thanks for the input! It’d be great to hear more feedback from people on edge cases like this since it affects the design and complexity of the implementation. Here’s another one:

  def one(cond: Boolean, fallback: Boolean): Boolean =
    cond || fallback

  def two(cond: Boolean, fallback: => Boolean): Boolean =
    cond || fallback

  def three(cond: Boolean, fallback: () => Boolean): Boolean =
    cond || fallback()

If cond is true and the boolean is short-circuited, should the second argument be marked as covered or not? The current behavior of scoverage marks the first two as covered but the last one as uncovered.

1 Like

I think that in an ideal world the 2nd argument would be marked not-covered in the last two cases but having only the last case not-covered is fine too. Since most users typically start with a simplistic “was the whole line hit” model, they generally need to do some digging in order to be able to understand that short-circuiting is going on at all. Conceptually, it is easier to notice that short-circuting would affect the last-case than the second case so it would occur to a user much faster than in the 2nd case.

1 Like

I’d honestly prefer having the second marked uncovered in all 3 cases. Aside from simplifying the mental model of when something gets marked and when it doesn’t, knowing if the fallback was never reached could help debug why a particular branch isn’t getting hit downstream.

I take the view that in all 3 cases the arguments to the function should be marked as covered, but the shortcircuited statement should be marked as not covered.

in my view the method declaration should be marked as covered if the method is executed at all. I don’t think the coverage tool should try and calculate whether or not a particular lazy parameter has been accessed.

however instrumentation at the call site should indicate whether or not the supplied byName parameter was executed or not.

one possible naive solution to alot of these issues would be to introduce an invokeAround method to the Invoker class and wrap any statements in one of those calls (with some filtering).

method calls are problematic, but one solution to that could be nArity variations of invokeAround

for example

val a = ""
lazy val b = 1
def c = false
foo(x => x + x, a, b, c)

could be transformed to

val a = invokeAround("", ...)
lazy val b = {
  invoked(...)
  invokeAround(1, ...)
}
def c =  {
   invoked(...)
   invokeAround(false)
}

invokeAround4(
  (_1,_2,_3,_4) => foo(_1,_2,_3,_4), 
  invokeAround(x => x + x),
  invokeAround(a),
  invokeAround(b),
  invokeAround(c), ...)

with the possiblity to drop some invoke arounds when a val is passed to a strict method parameter etc

sudo implementation of invokeAround

def invokeAround[A](a :=> A, id: number, file: String): A = {
  invoked(number, file)
  a
}
def invokedAround1[A, Res](f: A => Res, param1: A, id: number, file: String): Res = {
   // enforces strict evaluation of the parameters
   // if a throw happens during evaluation of param1 then this code won't be reached
   invoked(id, file)
   f(param1)
}

Sorry I should’ve been clearer, by “arguments” I meant, “arguments to the || operator”, not “method parameters”, but I get what you mean.

Looking at the code it seems that this idea is not necessary. Since it already uses a lifting based implementation: (was fun to think about though)

dotty.tools.dotc.typer.LiftCoverage$#liftForCoverage

Just want to say its appreciated how folks have focused on this item, which slipped through the cracks.

I think assumptions of positive intent can sometimes leave you burnt, I choose to assume positive intent from the community here.

The Scala compiler team, VirtusLab, and Scala Center have shown a real commitment to improving the tooling many have complained about.

I just wanted to say thanks.

13 Likes

Ditto!

1 Like

@smarter I’ve had a go at an implementation for shortcircuited boolean expressions (and a similar thing for numerical expressions). This numeric expressions are a bit overzealous, since it instruments everything that was selected for the expression.

Perhaps it would make sense for that to be the default and add special cases for:

  • Simple numeric expressions with only literals and val/var coefficients. (These can be instrumented as one thing)
  • Simple numeric expressions that make up an assignment. (The left and right handside can be instrumented as one thing)

The result of something like a < b && b > 0 || c is 3 instrumented expressions a < b, b > 0, c

1 Like

I have been a long-time user of scoverage for Scala 2. It does the job, however it’s not perfect. Small annoyances aside, my grievance has been that the process becomes really slow when the test coverage is turned on. At $work I’m thinking of dropping the coverage metrics entirely due to the build with tests taking close to an hour, which is bad if you’re waiting on it to finish in order to test the deployed artefacts. And I’m against running the test coverage for PRs for the same reason.

Not sure if there’s anything that anyone can do about it, maybe this is intrinsic to the JVM, but some performance improvements would be appreciated too.

3 Likes

The initial project (http://guillaume.martres.me/code_coverage.pdf) measured some significant performance improvements in the Scala 3 implementation compared to the Scala 2 one, but I don’t think anyone has benchmarked it recently. I think it should be possible to improve it further using invokedynamic to reduce the bytecode overhead (I believe jacoco does that too).