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

Java code coverage for sh_tests missing #5331

Closed
dhalperi opened this Issue Jun 5, 2018 · 8 comments

Comments

Projects
None yet
2 participants
@dhalperi
Copy link

dhalperi commented Jun 5, 2018

Description of the problem:

Copying from bazel-discuss thread per request:

We have a mix of JUnit tests and sh_tests that invoke a binary in a test scenario. bazel coverage //... --jobs 1 [0] works great for the unit tests, but does not generate a coverage.dat for the sh_tests.

Repo is open source: https://github.com/batfish/batfish@c6658e88 is base, and https://github.com/batfish/batfish/blob/c6658e88c8c9d48e393547281cc119d99b90670d/tests/parsing-tests/BUILD is the integration sh_test.

I tried figuring out how the Java coverage works under the hood, but I got fairly stymied. Any guidance?

[0] --jobs 1 workaround for crash: #4398

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Check out the linked repository at the linked commit and run bazel coverage --jobs 1 //.... Command log below:

➜  batfish git:(master) git checkout c6658e88c8c9d48e393547281cc119d99b90670d 
Note: checking out 'c6658e88c8c9d48e393547281cc119d99b90670d'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again. Example:

  git checkout -b <new-branch-name>

HEAD is now at c6658e88c Local interface in bgp session status (#1588)
➜  batfish git:(c6658e88c) bazel clean --expunge                                
INFO: Starting clean.
➜  batfish git:(c6658e88c) bazel coverage --jobs 1 //...
INFO: SHA256 (https://services.gradle.org/distributions/gradle-4.1-bin.zip) = d55dfa9cfb5a3da86a1c9e75bb0b9507f9a8c8c100793ccec7beb6e259f9ed43
INFO: Using default value for --instrumentation_filter: "//projects,//tests/parsing-tests".
INFO: Override the above default with --instrumentation_filter
INFO: Analysed 59 targets (190 packages loaded).
INFO: Found 52 targets and 7 test targets...
INFO: Elapsed time: 289.417s, Critical Path: 47.31s
INFO: 324 processes: 261 darwin-sandbox, 13 local, 50 worker.
INFO: Build completed successfully, 500 total actions
//projects/allinone:allinone_tests                                       PASSED in 6.0s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/allinone/allinone_tests/coverage.dat
//projects/batfish:batfish_tests                                         PASSED in 23.8s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/batfish/batfish_tests/coverage.dat
//projects/batfish-client:client_tests                                   PASSED in 2.1s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/batfish-client/client_tests/coverage.dat
//projects/batfish-common-protocol:common_tests                          PASSED in 3.1s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/batfish-common-protocol/common_tests/coverage.dat
//projects/coordinator:coordinator_tests                                 PASSED in 5.7s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/coordinator/coordinator_tests/coverage.dat
//projects/question:question_tests                                       PASSED in 1.8s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/question/question_tests/coverage.dat
//tests/parsing-tests:ref_tests                                          PASSED in 25.0s

Executed 7 out of 7 tests: 7 tests pass.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see wINFO: Build completed successfully, 500 total actions

Note that there were 7 tests run, only 6 coverage.dat files produced. The 6 tests are junit_tests and the 7th is a sh_test.

Note that I would actually be (very pleasantly) surprised if this worked as-is... I'd expect I might have to do something in the generated sh_test to indicate I wanted Java coverage. I just don't know what that something might be...

What operating system are you running Bazel on?

macOS High Sierra 10.13.5 (17F77)

What's the output of bazel info release?

release 0.14.0-homebrew

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

N/A

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

N/A I believe.

Have you found anything relevant by searching the web?

Any other information, logs, or outputs that you want to share?

@dhalperi

This comment has been minimized.

Copy link

dhalperi commented Jun 5, 2018

Can't assign with my permissions, but tagging @iirina

@iirina iirina self-assigned this Jun 6, 2018

@iirina

This comment has been minimized.

Copy link
Contributor

iirina commented Jul 11, 2018

@dhalperi can you describe what the sh_test is doing under the hood? How is it invoking the Java binary?

@iirina

This comment has been minimized.

Copy link
Contributor

iirina commented Jul 11, 2018

To add more context: sh_test currently doesn't have coverage support but it can be easily added. In order for it to work you would need to declare the binary as a dependency of the test, either as srcs, deps or data. The binary should also be the output of a Java rule.

@dhalperi

This comment has been minimized.

Copy link

dhalperi commented Jul 11, 2018

@iirina The links to the actual code are in the initial issue message and bazel-discuss thread. To re-summarize, and provide links to current implementation:

There's a java_binary called allinone_main, which has a java_library called allinone as a runtime dependency link to BUILD:

java_binary(
    name = "allinone_main",
    main_class = "org.batfish.allinone.Main",
    runtime_deps = [
        ":allinone",
    ],
)

The sh_test just runs the given allinone_main java_binary with a particular command line. The allinone_main is a data dependency of the sh_test. (Since there are many of these form of test, we use a skylark macro to build them).

_CMD = """
ALL_FILES="$(rootpaths {allinone})"
BIN_FILE=$$(tr " " "\n" <<< $${{ALL_FILES}} | grep -v jar)
echo \
  $${{BIN_FILE}} \
    -coordinatorargs '"-periodassignworkms 5"' \
    -cmdfile $(location {commands}) > $@
"""

def ref_tests(name, commands, allinone = None, extra_deps = None, **kwargs):
    if allinone == None:
        allinone = "//projects/allinone:allinone_main"

    # Generate a shell script that will run allinone on given commands file
    cmd = _CMD.format(allinone = allinone, commands = commands)
    native.genrule(
        name = "gen_" + name + ".sh",
        outs = [name + ".sh"],
        cmd = cmd,
        srcs = [allinone, commands],
    )

    # Run the sh_test on the needed inputs.
    if extra_deps == None:
        extra_deps = []
    native.sh_test(
        name = name,
        srcs = [name + ".sh"],
        data = [
            allinone,
            commands,
        ] + extra_deps,
    )
@dhalperi

This comment has been minimized.

Copy link

dhalperi commented Jul 11, 2018

The current output of bazel coverage --jobs=1 //...:

➜  batfish git:(master) ✗ bazel coverage --jobs 1 //...       
Starting local Bazel server and connecting to it...
................
INFO: Using default value for --instrumentation_filter: "//projects,//tests".
INFO: Override the above default with --instrumentation_filter
INFO: Analysed 81 targets (202 packages loaded).
INFO: Found 66 targets and 15 test targets...
INFO: Elapsed time: 318.442s, Critical Path: 52.86s
INFO: 113 processes: 57 darwin-sandbox, 20 local, 36 worker.
INFO: Build completed successfully, 246 total actions
//projects/allinone:allinone_tests                                       PASSED in 13.8s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/allinone/allinone_tests/coverage.dat
//projects/batfish:batfish_tests                                         PASSED in 21.5s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/batfish/batfish_tests/coverage.dat
//projects/batfish-client:client_tests                                   PASSED in 2.0s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/batfish-client/client_tests/coverage.dat
//projects/batfish-common-protocol:common_tests                          PASSED in 2.4s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/batfish-common-protocol/common_tests/coverage.dat
//projects/coordinator:coordinator_tests                                 PASSED in 5.0s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/coordinator/coordinator_tests/coverage.dat
//projects/question:question_tests                                       PASSED in 2.1s
  /private/var/tmp/_bazel_dan/ffd2f268833102995456f3fc3b57cee1/execroot/batfish/bazel-out/darwin-fastbuild/testlogs/projects/question/question_tests/coverage.dat
//tests/aws:ref_tests                                                    PASSED in 14.1s
//tests/basic:ref_tests                                                  PASSED in 24.2s
//tests/java-smt:ref_tests                                               PASSED in 24.2s
//tests/jsonpath-addons:ref_tests                                        PASSED in 15.2s
//tests/jsonpathtotable:ref_tests                                        PASSED in 15.6s
//tests/parsing-errors-tests:ref_tests                                   PASSED in 14.5s
//tests/parsing-tests:ref_tests                                          PASSED in 27.5s
//tests/roles:ref_tests                                                  PASSED in 16.3s
//tests/ui-focused:ref_tests                                             PASSED in 16.1s
@iirina

This comment has been minimized.

Copy link
Contributor

iirina commented Jul 12, 2018

Thank you for the additional information, it was very helpful! I have a fix in progress, should be out soon.

@bazel-io bazel-io closed this in 8fcc07a Jul 23, 2018

@dhalperi

This comment has been minimized.

Copy link

dhalperi commented Jul 23, 2018

Just pulled HEAD, indeed this works! Thanks!

Now all I'm missing is a way to aggregate all the coverage data :p. But you already have a plan for that, too, I believe.

@iirina

This comment has been minimized.

Copy link
Contributor

iirina commented Jul 24, 2018

You can use

--combined_report=lcov \
--coverage_report_generator=@bazel_tools//tools/test/LcovMerger/java/com/google/devtools/lcovmerger:Main

with Bazel @ HEAD and it should generate an aggregated report under

bazel-out/_coverage/_coverage_report.dat

werkt added a commit to werkt/bazel that referenced this issue Aug 2, 2018

Collect code coverage for binaries invoked via sh_test.
Fixes bazelbuild#5331.

Change-Id: Idb01a3f206ed37992f200f7e0e51ed9831262613

RELNOTES: Code coverage is collected for Java binaries invoked from sh_test.
PiperOrigin-RevId: 205654442
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment