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

Scoverage reports incorrect statement invocation count #120

Open
pawel-wiejacha opened this issue May 20, 2015 · 1 comment
Open

Scoverage reports incorrect statement invocation count #120

pawel-wiejacha opened this issue May 20, 2015 · 1 comment

Comments

@pawel-wiejacha
Copy link

Scoverage reports incorrect statement invocation count (Statement.count: Int).

Currently, for each statement, invocation count is either:

  • 0 if the statement was not hit at all or
  • 1 if the statement was hit more than once

I believe that reporting real statement invocation count is useful in some situations.
Often people check detailed coverage reports to improve their tests. For example, given:

                              // line invocation count:
val (foo, bar) = someLogic()  // 4
                              //
if (foo) branch1()            // 3
else branch2()                // 1
                              //
if (bar) branch3()            // 3
else branch4()                // 1
  • someone wrote 4 test cases to cover all possible paths, and want to check that each branch was executed at least twice
  • someone wants to check if his code is actually covered by unit tests, or if most of statements are hit by a single generic integration test that does not care about detailed checking
  • if someone wants to know how many tests hits given method/class

So the current binary information (hit/missed) may be not enough.

I know it's not a bug - Scoverage does it intentionally:

/**
* We record that the given id has been invoked by appending its id to the coverage
* data file. [...]
*/
class Invoker {
  def invoked(id: Int, dataDir: String): Unit = {
    // [sam] we can do this simple check to save writing out to a file.
    // This won't work across JVMs but since there's no harm in writing out the same id multiple
    // times since for coverage we only care about 1 or more, (it just slows things down to
    // do it more than once), anything we can do to help is good. This helps especially with code
    // that is executed many times quickly, eg tight loops.
    if (!ids.contains(dataDir, id)) // ...
  }

  // loads all the invoked statement ids from the given files
  def invoked(files: Seq[File]): Set[Int]
}

However, the Statement is ready to handle non-binary invocation count:

class Statement(... var count:Int ... ) {
  def invoked(): Unit = count = count + 1
}

I would like to fix this issue and create a PR, but I'm not sure if you want the fix:)

There are several possibilities to fix it:

  1. Simple, exact, but with poor performance:
    • replace def invoked(files: Seq[File]): Set[Int] with def invoked(files: Seq[File]): List[Int]
    • remove deduplication in Invoker.invoke- write statement id to a measurement file in every invocation
  2. Simple, exact when it's necessary, with good performance:
    • replace def invoked(files: Seq[File]): Set[Int] with def invoked(files: Seq[File]): Map[Id, Count]
    • in Invoker.invoke, write() + flush() statement id to a file only when id is encountered:
      • for the first time - to get correct code coverage
      • for the next 50 times - to get correct invocation count when exact information makes a difference
      • for the 64, 128, 256, 512, ... time - to avoid performance reduction, while keeping report invocation count with error smaller than 100%
  3. Complicated, exact, with good performance:
    • just like the previous one, instrument java.lang.{Thread, ThreadGroup, Runtime} to dump in-memory counters during Thread.join(), Thread.stop(), etc.

I think the option 2 is the best one and change would be less than 50 LOC.

What do you think?

@NoamShaish
Copy link

Any updates regarding this issue?
I am trying to incorporate scoverage with mutation testing and the hit count is an important optimisation metric.

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

No branches or pull requests

3 participants