You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "Damir Murat (Jira)" <ji...@apache.org> on 2022/12/17 09:47:00 UTC

[jira] [Updated] (GROOVY-10878) Improve JaCoCo's branch code coverage of a Groovy assert statement

     [ https://issues.apache.org/jira/browse/GROOVY-10878?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Damir Murat updated GROOVY-10878:
---------------------------------
    Description: 
At the moment, there is no way to have full branch coverage for Groovy {{assert}} statements. In a larger project, this is very distractive when trying to find the pieces of actual logic that should be better covered with tests. I believe a slight change in {{assert}} statement code generation can significantly improve the situation.

JaCoCo has long-standing issues with covering calls of methods that throw exceptions. When such methods are called inside of, {{if/else}} branches, for example, the result is partial coverage reported for those branches.

However, there is a JaCoCo idiom ([https://github.com/jacoco/jacoco/issues/370#issuecomment-267854179|http://example.com/]) that can be used to avoid uncovered code in those cases. The basic idea is to create and return an exception from a called method and throw that exception from a caller, like in:
{code:java}
void fail() {
  throw create();
}

RuntimeException create() {
  return new RuntimeException();
}
{code}
How this relates to the Groovy {{assert}} statement? For example, for a simple {{assert}} statement like
{code:java}
assert firstName != null
{code}
Groovy generates something like
{code:java}
ValueRecorder var1 = new ValueRecorder();

try {
  String var10000 = this.firstName;
  var1.record(var10000, 8);
  var1.record(var10000, 8);
  if (var10000 != null) {
    var1.clear();
  } else {
    ScriptBytecodeAdapter.assertFailed(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
  }

} catch (Throwable var3) {
  var1.clear();
  throw var3;
}
{code}
The problem with generated code is a
{code:java}
ScriptBytecodeAdapter.assertFailed(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
{code}
method call. Inside that method, an exception is created and thrown. Since JaCoCo cannot cover that line completely, the branch
{code:java}
if (var10000 != null)
{code}
will be reported as partially covered.

To avoid those issues, {{ScriptBytecodeAdapter.assertFailed()}} can be adapted (or a new method can be introduced like in the example below) to return the exception instead of throwing it. And then, the calling generated code can throw that returned exception:
{code:java}
try {
  ...
  if (var10000 != null) {
    ...
  } else {
    Throwable throwable = ScriptBytecodeAdapter.createAssertionError(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
    throw throwable
  }

} catch (Throwable var3) {
  ...
}
{code}
I have a small project demonstrating the issue and a possible solution here:
[https://github.com/dmurat/groovy-assert-jacoco-coverage-problem|http://example.com/]

Tnx

  was:
At the moment, there is no way to have full branch coverage for Groovy {{assert}} statements. In a larger project, this is very distractive when trying to find the pieces of actual logic that should be better covered with tests. I believe a slight change in {{assert}} statement code generation can significantly improve the situation.

JaCoCo has long-standing issues with covering calls of methods that throw exceptions.
When such methods are called inside of, {{if/else}} branches, for example, the result is
partial coverage reported for those branches.

However, there is a JaCoCo idiom ([https://github.com/jacoco/jacoco/issues/370#issuecomment-267854179|http://example.com]) that can be used to avoid uncovered code in those cases. The basic idea is to create and return an exception from a called method and throw that exception from a caller, like in:
{code:java}
void fail() {
  throw create();
}

RuntimeException create() {
  return new RuntimeException();
}
{code}

How this relates to the Groovy {{assert}} statement? For example, for a simple {{assert}} statement like

{code:java}
assert firstName != null
{code}

Groovy generates something like

{code:java}
ValueRecorder var1 = new ValueRecorder();

try {
  String var10000 = this.firstName;
  var1.record(var10000, 8);
  var1.record(var10000, 8);
  if (var10000 != null) {
    var1.clear();
  } else {
    ScriptBytecodeAdapter.assertFailed(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
  }

} catch (Throwable var3) {
  var1.clear();
  throw var3;
}
{code}

The problem with generated code is a

{code:java}
ScriptBytecodeAdapter.assertFailed(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
{code}
method call. Inside that method, an exception is created and thrown. Since JaCoCo cannot cover that line completely, the branch
{code:java}
if (var10000 != null)
{code}
will be reported as partially covered.

To avoid those issues, {{ScriptBytecodeAdapter.assertFailed()}} can be adapted (or a new method can be introduced like in the example below) to return the exception instead of throwing it. And then, the calling generated code can throw that returned exception:

{code:java}
try {
  ...
  if (var10000 != null) {
    ...
  } else {
    Throwable throwable = ScriptBytecodeAdapter.createAssertionError(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
    throw throwable
  }

} catch (Throwable var3) {
  ...
}
{code}

I have a small project demonstrating the issue and a possible solution here:
[https://github.com/dmurat/groovy-assert-jacoco-coverage-problem|http://example.com]

Tnx


> Improve JaCoCo's branch code coverage of a Groovy assert statement
> ------------------------------------------------------------------
>
>                 Key: GROOVY-10878
>                 URL: https://issues.apache.org/jira/browse/GROOVY-10878
>             Project: Groovy
>          Issue Type: Improvement
>    Affects Versions: 4.0.6
>            Reporter: Damir Murat
>            Priority: Major
>
> At the moment, there is no way to have full branch coverage for Groovy {{assert}} statements. In a larger project, this is very distractive when trying to find the pieces of actual logic that should be better covered with tests. I believe a slight change in {{assert}} statement code generation can significantly improve the situation.
> JaCoCo has long-standing issues with covering calls of methods that throw exceptions. When such methods are called inside of, {{if/else}} branches, for example, the result is partial coverage reported for those branches.
> However, there is a JaCoCo idiom ([https://github.com/jacoco/jacoco/issues/370#issuecomment-267854179|http://example.com/]) that can be used to avoid uncovered code in those cases. The basic idea is to create and return an exception from a called method and throw that exception from a caller, like in:
> {code:java}
> void fail() {
>   throw create();
> }
> RuntimeException create() {
>   return new RuntimeException();
> }
> {code}
> How this relates to the Groovy {{assert}} statement? For example, for a simple {{assert}} statement like
> {code:java}
> assert firstName != null
> {code}
> Groovy generates something like
> {code:java}
> ValueRecorder var1 = new ValueRecorder();
> try {
>   String var10000 = this.firstName;
>   var1.record(var10000, 8);
>   var1.record(var10000, 8);
>   if (var10000 != null) {
>     var1.clear();
>   } else {
>     ScriptBytecodeAdapter.assertFailed(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
>   }
> } catch (Throwable var3) {
>   var1.clear();
>   throw var3;
> }
> {code}
> The problem with generated code is a
> {code:java}
> ScriptBytecodeAdapter.assertFailed(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
> {code}
> method call. Inside that method, an exception is created and thrown. Since JaCoCo cannot cover that line completely, the branch
> {code:java}
> if (var10000 != null)
> {code}
> will be reported as partially covered.
> To avoid those issues, {{ScriptBytecodeAdapter.assertFailed()}} can be adapted (or a new method can be introduced like in the example below) to return the exception instead of throwing it. And then, the calling generated code can throw that returned exception:
> {code:java}
> try {
>   ...
>   if (var10000 != null) {
>     ...
>   } else {
>     Throwable throwable = ScriptBytecodeAdapter.createAssertionError(AssertionRenderer.render("assert firstName != null", var1), (Object)null);
>     throw throwable
>   }
> } catch (Throwable var3) {
>   ...
> }
> {code}
> I have a small project demonstrating the issue and a possible solution here:
> [https://github.com/dmurat/groovy-assert-jacoco-coverage-problem|http://example.com/]
> Tnx



--
This message was sent by Atlassian Jira
(v8.20.10#820010)