[Code Coverage] Add a tool to check test coverage of a patch

[xgupta]

This Python script creates a patch from the HEAD commit, extracts modified or added source files, test case files, and source code lines, adds coverage instrumentation for the affected source files, runs Lit tests, and records which test cases cause each counter to be executed. Then report the number of test cases executing the counter and the number of test cases executing the counter that are also changed in some way by the patch. Thus providing the developer the information of inadequately tested source lines.

[xgupta]

Moved the code review from Phabricator (https://reviews.llvm.org/D158864) since quite a long we had deprecated Phabricator.

[xgupta]

Hi All,

I have run the tool on GitHub CI for patch - #72273.

There are twenty-eight lines not covered by any test case -
https://github.com/xgupta/llvm-project/actions/runs/6994498992/job/19028333944#step:5:2609

This is a patch with .ll test case and opt binary. Tomorrow I will run it for a patch with .c & .cpp test case and clang binary and share the result.

[xgupta]

Hi,

I have run it for patch - #71894 which is for the clang binary and .c test cases.

There are twenty lines not covered by any test case -
https://github.com/xgupta/llvm-project/actions/runs/6994512086/job/19028363714#step:5:3726

Here I have to change the way to run the tool -
From
python3 git-check-coverage.py -b build bin/opt llvm/test

To
python3 git-check-coverage.py -b build bin/clang clang/test

[xgupta]

I have tested it on an llc binary and .mir test case patch - #72388.

It works and shows three lines are not covered for two source files on standard output and there is a log file as artifect which has detailed log of coverage data - https://github.com/xgupta/llvm-project/suites/18461817041/artifacts/1070751301.

https://github.com/xgupta/llvm-project/actions/runs/6977269225/job/18986837631#step:5:2602

Also run with ld.lld binary and .s test case for patch - #71248
https://github.com/xgupta/llvm-project/actions/runs/6994370287/job/19028052559

[xgupta]

I realize that it is not working for unit tests.

So I wrote a separate function to run them but unlike unit tests of clang, llvm unit tests are not showing up with build/bin/llvm-lit --show-tests llvm/test which is another thing to investigate.

But I think it should be fine to review and commit this version and then proceed for unit tests coverage in the next step.

def run_modified_unit_tests(build_dir, patch_path, tests):
    """Read the patch file, identify modified and added test cases, and
    then execute each of these test cases."""

    try:
        # Get the list of modified and added test cases from the patch
        with open(patch_path, "r") as patch_file:
            patch_diff = patch_file.read()

        modified_tests = []

        custom_print()
        # Use regular expressions to find modified test cases with ".ll|.c|.cpp" extension
        for match in re.finditer(
            r"^\+\+\+ [ab]/(.*\.(ll|mir|mlir|fir|c|cpp|f90|s|test))$",
            patch_diff,
            re.MULTILINE,
        ):
            test_file = match.group(1)
            # Get the current working directory
            cwd = os.getcwd()

            # Build the full file path dynamically by going two steps back from cwd
            full_test_file = os.path.join(
                os.path.dirname(cwd), "llvm-project", test_file
            )

            # Extract the first three directories from the test_file path
            first_three_dirs = os.path.join(*test_file.split(os.path.sep)[:3])

            matching_test_paths = [
                test_path for test_path in tests if first_three_dirs in test_path
            ]
            if matching_test_paths:
                custom_print()
                custom_print("Unit test file in the patch:", test_file)
                custom_print("Full unit test file path:", full_test_file)
                custom_print("Matching unit test in tests:", matching_test_paths[0])

                # Capture the first matching test path
                modified_test_path = os.path.dirname(
                    os.path.dirname(matching_test_paths[0])
                )

                # Extract the file name (excluding the extension) from test_file
                file_name = os.path.splitext(os.path.basename(test_file))[0]

                # Extract the last directory name (excluding the extension) from test_file
                last_directory = os.path.splitext(
                    os.path.basename(os.path.dirname(test_file))
                )[0]

                # Add "Tests" to the last_directory
                last_directory_with_tests = f"{last_directory}Tests"

                # Set LLVM_PROFILE_FILE environment variable
                llvm_profile_file = os.path.join(
                    os.path.dirname(modified_test_path), f"{file_name}.profraw"
                )
                os.environ["LLVM_PROFILE_FILE"] = llvm_profile_file

                cwd = os.getcwd()
                os.chdir(build_dir)
                subprocess.check_call(["ninja", last_directory_with_tests])
                os.chdir(cwd)

                # Check if the file name is starts with +++
                if match.group(0).startswith("+++"):
                    modified_tests.append(full_test_file)

                    # Run each modified test case
                    custom_print("")
                    custom_print(f"Running modified test cases with Ninja target {last_directory_with_tests}:")
                    subprocess.check_call(
                        [modified_test_path, f"--gtest_filter={file_name}*"]
                    )
                    custom_print("Test case executed:", full_test_file)

        if not modified_tests:
            custom_print("No modified unit tests found in the patch.")
            return

    except subprocess.CalledProcessError as e:
        custom_print("Error while running modified tests:", e)
        sys.exit(1)

    except Exception as ex:
        custom_print("Error:", ex)
        sys.exit(1)

[xgupta]

Actually, the above issue is because of -DLLVM_BUILD_TESTS=ON flag, using that in cmake, gives the list of all unit tests.

I have llvm unit tests patch - #71952
https://github.com/xgupta/llvm-project/actions/runs/7002086614/job/19045204411

I have clang unit test patch - #71672
https://github.com/xgupta/llvm-project/actions/runs/7002113619/job/19045279617

[tstellar]

I don't know much about coverage, but I don't see any reason not to add this.

[xgupta]

Actually, added a few reviewers who are contributing to LLVM Python code, just to make sure it is following the correct coding practices. I haven't worked on Python before this.

[tru]

I know we are not doing this right now, but I wonder if we should have tests for python scripts as well so that we can run it with our supported python version and see if it works, because it can be hard to spot in a review.

I will make a sweep on the code later today.

[boomanaiden154]

I know we are not doing this right now, but I wonder if we should have tests for python scripts as well so that we can run it with our supported python version and see if it works, because it can be hard to spot in a review.

I am very much in favor of starting to add tests for these utilities. I wouldn't block this patch on having tests, but it's definitely a good practice in general. It's on my todo list to get tests up and running for git-clang-format.

[boomanaiden154]

Quite a few nits, lots of other comments on my initial pass.

This looks like it could be pretty useful. Thanks for putting this together! I'm envisioning this as an on demand job that can be run on a PR with a rich representation reported in a comment (something to be added later) could be a pretty good litmus check for a lot of things.

One question: It seems like this doesn't currently cover cases where only source files or only test files are touched. Just looking at test files might be difficult as there's no way to know what to report. But, looking at just source files I think would be doable (would just require running the whole test suite, not sure how expensive this is) and would be really useful for refactorings and probably other cases.