Feature Request: Code coverage support inside Xcode

[marcelofabri]

I didn't see this explicitly being tracked anywhere, so decided to create an issue.

It'd be great to have code coverage support inside Xcode, so engineers could look at coverage reports after running tests and having it shown inside the editor.

[brentleyjones]

FYI, I've given this a "Post 1.0" milestone because I know some of the difficulty in getting this working correctly, and I don't think it should block the 1.0 release, not because I don't think it's important.

[sanju-naik]

Just checking in, were we able to make any progress on this? It would be great to have this feature 👍

[keith]

I implemented a strategy for this which works for Lyft, so I figured I'd post it here in case someone wants to figure out how to generalize it.

1. I added a patch to rules_swift to pass a -coverage-prefix-map EXEC_ROOT=SOURCE_ROOT, I was able to compute the source root by getting the 'realpath' of a known directory in the execroot, which only works with sandboxing disabled (I think, I didn't test because we use that anyways for developer builds)
2. I created a string_flag which I pass on the command line from the script that builds specific test targets like --//:enable_coverage_in_xcode=TARGET_NAME
3. In our ios_unit_test target macro I created a config_setting for if the TARGET_NAME in that flag is the current target name
4. Based on this flag I set a few attributes to enable things in the source module. Specifically I disable the default prefix remaps that remove the absolute paths, and I enable coverage only for this target (to preserve caching for the dependent targets):

copts = select({
    ":coverage_in_xcode": ["-Xwrapped-swift=-lyft-coverage-hack"],
    "//conditions:default": [],
})
features = select({
    ":coverage_in_xcode": [
        "-swift.coverage_prefix_map",
        "-swift.file_prefix_map",
        "swift.coverage",
    ],
    "//conditions:default": [],
})
linkopts = select({
    ":coverage_in_xcode": ["-fprofile-instr-generate"],
    "//conditions:default": [],
})

Depending on your project setup a downside of this approach might be that it only collects coverage for the specific module you're testing (although you could potentially re-organize the config_settings to make it apply to more modules). This works fine for our project since we don't want you to consider how your test target affects code coverage in your dependencies anyways, so if you want coverage for those targets you should test them directly instead.

This of course means the top level module you're testing will not get cache hits, but we're willing to make this tradeoff for the usefulness of this IDE feature combined with the fact that if you're testing this module you're likely changing it and invalidating the caches anyways.

Also there's the downside that setting and unsetting the --//:enable_coverage_in_xcode=TARGET_NAME flag thrashes the analysis cache.

I spent quite a while trying to come up with a tool that would instead rewrite the binary with absolute path coverage data after the fact, but that has proven to be quite difficult given implementation details of the coverage mapping format.

[erneestoc]

"-Xwrapped-swift=-lyft-coverage-hack"

what does this do?

I added a patch to rules_swift to pass a -coverage-prefix-map EXEC_ROOT=SOURCE_ROOT

you made a patch for rules_swift, or just internally for Lyft?

I'd like to give this a try soon - for my specific project, I think I might duplicate my test targets with a modified version of the target to test, and use that on the xcodeproj schemas - or dk - I guess I'll just try to make it work first 😅

[keith]

what does this do?

The functionality is what I described in #1 above

you made a patch for rules_swift, or just internally for Lyft?

Just for Lyft, the patch is definitely a hack, so I don't really think we should add it to rules_swift. I didn't post it here since I don't really want to be on the hook for maintaining it either 🙃

[keith]

For reference here's the patch:

diff --git a/tools/worker/swift_runner.cc b/tools/worker/swift_runner.cc
index 56d1552..fe8f3ed 100644
--- a/tools/worker/swift_runner.cc
+++ b/tools/worker/swift_runner.cc
@@ -291,6 +291,15 @@ bool SwiftRunner::ProcessArgument(
         // without breaking hermiticity.
         consumer("-coverage-prefix-map");
         consumer(std::filesystem::current_path().string() + "=.");
+      } else if (new_arg == "-lyft-coverage-hack") {
+        auto cwd = std::filesystem::current_path();
+        // The bazel exec root is a normal directory, but inside of it there are
+        // symlinks to our source tree. This fetches the true path of a known
+        // directory in order to get the actual source root of the project. This
+        // should only work with sandboxing disabled.
+        auto target_path = std::filesystem::canonical(cwd / "Modules").parent_path();
+        consumer("-coverage-prefix-map");
+        consumer(std::filesystem::current_path().string() + "=" + target_path.string());
         changed = true;
       } else if (new_arg == "-file-prefix-pwd-is-dot") {
         // Replace the $PWD with . to make the paths relative to the workspace

Note that Modules is specific to Lyft and you'd have to change that to something that will always exist in builds in your setup instead

[brentleyjones]

You can probably use the root BUILD file instead of the /Modules directory.

[keith]

Do those get symlinked? Also BUILD vs BUILD.bazel might be an issue

[brentleyjones]

They do. You might also not have a root level one. But just saying, if you do, it's probably the best option.