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

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

Open
Rd4dev opened this issue Aug 22, 2024 · 0 comments
Open

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

Rd4dev opened this issue Aug 22, 2024 · 0 comments
Labels
bug End user-perceivable behaviors which are not desirable.

Comments

@Rd4dev
Copy link
Collaborator

Rd4dev commented Aug 22, 2024

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

@Rd4dev Rd4dev added bug End user-perceivable behaviors which are not desirable. triage needed labels Aug 22, 2024
@Rd4dev Rd4dev changed the title [BUG]: Code coverage fails to capture termination statements [BUG]: Code coverage fails to capture execution flow interruptions Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug End user-perceivable behaviors which are not desirable.
Development

No branches or pull requests

2 participants