Skip to content

[BUG]: Code coverage fails to capture execution flow interruptions #5506

@Rd4dev

Description

@Rd4dev

Describe the bug

It is observed that lines of code involving termination statements - such as exitProcess(1), assertion failures - such as assertThat(it).isNotPending() and flow interruption statements - such as break are not marked as covered even though they are executed.

This appears to be a known issue with JaCoCo. According to JaCoCo documentation, source code lines that involve exceptions or abrupt termination sequences show no coverage because the control flow is interrupted. JaCoCo FAQ Page

As per documentation:

Source code lines with exceptions show no coverage. Why?

JaCoCo determines code execution with so called probes. Probes are inserted into the control flow at certain positions. Code is considered as executed when a subsequent probe has been executed. In case of exceptions such a sequence of instructions is aborted somewhere in the middle and the corresponding lines of source code are not marked as covered.

Steps To Reproduce

  1. Termination statements:

Consider the RetrieveChangedFiles.kt file with the following termination statement:

fun main(args: Array<String>) {
  if (args.size < 5) {
    println(
      "Usage: bazel run //scripts:retrieve_changed_files --" +
        " <encoded_proto_in_base64> <path_to_bucket_name_output_file>" +
        " <path_to_file_list_output_file> <path_to_test_target_list_output_file>"
    )
    println("Exiting...")
    exitProcess(1)
  }
  ...
}

Generate code coverage report for the file RetrieveChangedFiles.kt:

bazel run //scripts:run_coverage -- $(pwd) scripts/src/java/org/oppia/android/scripts/ci/RetrieveChangedFiles.kt

Review the generated HTML code coverage report for RetrieveChangedFiles.kt:

image

  1. Assertion failures:

With the DataProviderTestMonitor.kt, it is observed that the lines,

fun <T> ensureDataProviderExecutes(dataProvider: DataProvider<T>) {
    // Waiting for a result is the same as ensuring the conditions are right for the provider to
    // execute (since it must return a result if it's executed, even if it's pending).
    val monitor = createMonitor(dataProvider)
    monitor.waitForNextResult().also {
        monitor.stopObservingDataProvider()
    }.also {
        // There must be an actual result for the provider to be successful.
        println("Asserting...")
        assertThat(it).isNotPending()
        println("After Asserting...")
    }
}

generate a coverage report of:

image

The coverage report indicates that the lines with assertThat and println("After Asserting...") are not hit, which suggests they are not being executed during the test. Based on the test behavior, it seems that the assertion fails and throws an exception, interrupting the flow, causing the subsequent lines to be skipped.

  1. Break:

The finally block in SurveyProgressController.kt was reported as uncovered. The current code snippet is:

  is ControllerMessage.FinishSurveySession -> {
    try {
      controllerState.completeSurveyImpl(message.callbackFlow)
    } finally {
      println("Finally...")
      // Ensure the actor ends since the session requires no further message processing.
      break
    }
  }

The report with break statement:

  • The presence of the break statement results in partial coverage for the finally block.

image

The report without break statement:

  • Commenting out the break statement results in full coverage of the finally block.

image

The partial coverage caused by the break statement suggests a potential alignment with the previously noted exception gap with JaCoCo.

Expected Behavior

The lines of code containing the termination statement (exitProcess(1)), assertThat(it).isNotPending(), break should be marked as covered if they are executed during a test run.

Additional Context

Other files that exhibit the behavior as (1 - Termination statements):

I suspect that the following code in PinPasswordActivityPresenter may also fall into the same category of termination statements.

fun handleOnDestroy() {
  if (::alertDialog.isInitialized && alertDialog.isShowing) {
    alertDialog.dismiss()
  }

  if (confirmedDeletion) {
    confirmedDeletion = false

    // End the process forcibly since the app is not designed to recover from major on-disk state
    // changes that happen from underneath it (like deleting all profiles).
    exitProcess(0)
  }
}

However, due to compatibility issues with code coverage tools (see tracking issue #5481), this file is currently exempted from coverage, making it difficult to determine the exact coverage details.

Bazel Version

6.5.0

What device/emulator are you using?

No response

Which Android version is your device/emulator running?

No response

Which version of the Oppia Android app are you using?

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    Impact: MediumModerate perceived user impact (non-blocking bugs and general improvements).Work: LowSolution is clear and broken into good-first-issue-sized chunks.bugEnd user-perceivable behaviors which are not desirable.

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions