Skip to content

Commit

Permalink
Introduce reproducible branch-based coverage
Browse files Browse the repository at this point in the history
Reports may now be build via:

```nix
nix build .#nativelinkCoverageForHost
```

The `result` symlink then contains the contents of a webpage to view the
existing reports.

Reports are built in release mode to closely resemble production coverage
of the testsuite. This also means that most worker tests are ignored via
a new `nix` feature to make the testsuite run in nix sandboxes. It's not
ideal, but accurately reflects our production coverage guarantees. A
future resolution for this might be  to implement more elaborate mocking
functionality for `nativelink-worker`.

Coverage leverages nix caching, but not Bazel caching. While Bazel has
builtin support for coverage, the reports were not satisfactory as they
rely on outdated gcov toolchains with vague hermeticity guarantees and
unsatisfactory implementations for llvm-cov-based workflows (e.g. no
branch-based coverage for rust and no story around coverage for
heterogeneous code). Mid-term we should implement "fast development"
coverage via Bazel alongside the "slow production" coverage via Nix.

As next steps we should continuously publish the generated HTML pages
via the web infrastructure and add hooks to report regressions.
  • Loading branch information
aaronmondal committed Sep 29, 2024
1 parent 73dbf59 commit dae772a
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 3 deletions.
10 changes: 10 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -468,3 +468,13 @@ most automatically generated changelogs provide.

NativeLink Code of Conduct is available in the
[CODE_OF_CONDUCT](https://github.com/tracemachina/nativelink/tree/main/CODE_OF_CONDUCT.md) file.

## Generating code coverage

You can generate branch-based coverage reports via:

```
nix run .#nativelinkCoverageForHost
```

The `result` symlink contains a webpage with the visualized report.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ name = "nativelink"
enable_tokio_console = [
"nativelink-util/enable_tokio_console"
]
nix = [
"nativelink-worker/nix"
]

[dependencies]
nativelink-error = { path = "nativelink-error" }
Expand Down
41 changes: 40 additions & 1 deletion flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,21 @@
];
};

nightlyRustFor = p:
p.rust-bin.nightly.${nightly-rust-version}.default.override {
extensions = ["llvm-tools"];
targets = [
"${nixSystemToRustTriple p.stdenv.targetPlatform.system}"
];
};

craneLibFor = p: (crane.mkLib p).overrideToolchain stableRustFor;
nightlyCraneLibFor = p: (crane.mkLib p).overrideToolchain nightlyRustFor;

src = pkgs.lib.cleanSourceWith {
src = (craneLibFor pkgs).path ./.;
filter = path: type:
(builtins.match "^.*(data/SekienSkashita\.jpg|nativelink-config/README\.md)" path != null)
(builtins.match "^.*(data/SekienAkashita\.jpg|nativelink-config/README\.md)" path != null)
|| ((craneLibFor pkgs).filterCargoSources path type);
};

Expand Down Expand Up @@ -184,6 +193,7 @@

# Additional target for external dependencies to simplify caching.
cargoArtifactsFor = p: (craneLibFor p).buildDepsOnly (commonArgsFor p);
nightlyCargoArtifactsFor = p: (craneLibFor p).buildDepsOnly (commonArgsFor p);

nativelinkFor = p:
(craneLibFor p).buildPackage ((commonArgsFor p)
Expand Down Expand Up @@ -334,6 +344,34 @@
os = "linux";
};
};

nativelinkCoverageFor = p: let
coverageArgs =
(commonArgsFor p)
// {
# TODO(aaronmondal): For some reason we're triggering an edgecase where
# mimalloc builds against glibc headers in coverage
# builds. This leads to nonexistend __memcpy_chk and
# __memset_chk symbols if fortification is enabled.
# Our regular builds also have this issue, but we
# should investigate further.
hardeningDisable = ["fortify"];
};
in
(nightlyCraneLibFor p).cargoLlvmCov (coverageArgs
// {
cargoArtifacts = nightlyCargoArtifactsFor p;
cargoExtraArgs = builtins.concatStringsSep " " [
"--all"
"--locked"
"--features nix"
"--branch"
"--ignore-filename-regex '.*(genproto|vendor-cargo-deps|crates).*'"
];
cargoLlvmCovExtraArgs = "--html --output-dir $out";
});

nativelinkCoverageForHost = nativelinkCoverageFor pkgs;
in rec {
_module.args.pkgs = let
nixpkgs-patched = (import self.inputs.nixpkgs {inherit system;}).applyPatches {
Expand Down Expand Up @@ -366,6 +404,7 @@
lre-cc
native-cli
nativelink
nativelinkCoverageForHost
nativelink-aarch64-linux
nativelink-debug
nativelink-image
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "nativelink-metric-macro-derive"
version = "0.4.0"
version = "0.5.3"
edition = "2021"

[lib]
Expand Down
3 changes: 3 additions & 0 deletions nativelink-worker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ name = "nativelink-worker"
version = "0.5.3"
edition = "2021"

[features]
nix = []

[dependencies]
nativelink-error = { path = "../nativelink-error" }
nativelink-proto = { path = "../nativelink-proto" }
Expand Down
1 change: 1 addition & 0 deletions nativelink-worker/tests/local_worker_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ fn make_temp_path(data: &str) -> String {
)
}

#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn platform_properties_smoke_test() -> Result<(), Error> {
let mut platform_properties = HashMap::new();
Expand Down
9 changes: 9 additions & 0 deletions nativelink-worker/tests/running_actions_manager_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,7 @@ async fn upload_files_from_above_cwd_test() -> Result<(), Box<dyn std::error::Er

// Windows does not support symlinks.
#[cfg(not(target_family = "windows"))]
#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn upload_dir_and_symlink_test() -> Result<(), Box<dyn std::error::Error>> {
const WORKER_ID: &str = "foo_worker_id";
Expand Down Expand Up @@ -1321,6 +1322,7 @@ async fn cleanup_happens_on_job_failure() -> Result<(), Box<dyn std::error::Erro
Ok(())
}

#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn kill_ends_action() -> Result<(), Box<dyn std::error::Error>> {
const WORKER_ID: &str = "foo_worker_id";
Expand Down Expand Up @@ -1429,6 +1431,7 @@ async fn kill_ends_action() -> Result<(), Box<dyn std::error::Error>> {
// The wrapper script will print a constant string to stderr, and the test itself will
// print to stdout. We then check the results of both to make sure the shell script was
// invoked and the actual command was invoked under the shell script.
#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn entrypoint_does_invoke_if_set() -> Result<(), Box<dyn std::error::Error>> {
#[cfg(target_family = "unix")]
Expand Down Expand Up @@ -1572,6 +1575,7 @@ exit 0
Ok(())
}

#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn entrypoint_injects_properties() -> Result<(), Box<dyn std::error::Error>> {
#[cfg(target_family = "unix")]
Expand Down Expand Up @@ -1747,6 +1751,7 @@ exit 0
Ok(())
}

#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn entrypoint_sends_timeout_via_side_channel() -> Result<(), Box<dyn std::error::Error>> {
#[cfg(target_family = "unix")]
Expand Down Expand Up @@ -2268,6 +2273,7 @@ async fn action_result_has_used_in_message() -> Result<(), Box<dyn std::error::E
Ok(())
}

#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn ensure_worker_timeout_chooses_correct_values() -> Result<(), Box<dyn std::error::Error>> {
const WORKER_ID: &str = "foo_worker_id";
Expand Down Expand Up @@ -2681,6 +2687,7 @@ async fn worker_times_out() -> Result<(), Box<dyn std::error::Error>> {
Ok(())
}

#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn kill_all_waits_for_all_tasks_to_finish() -> Result<(), Box<dyn std::error::Error>> {
const WORKER_ID: &str = "foo_worker_id";
Expand Down Expand Up @@ -2841,6 +2848,7 @@ async fn kill_all_waits_for_all_tasks_to_finish() -> Result<(), Box<dyn std::err

/// Regression Test for Issue #675
#[cfg(target_family = "unix")]
#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn unix_executable_file_test() -> Result<(), Box<dyn std::error::Error>> {
const WORKER_ID: &str = "foo_worker_id";
Expand Down Expand Up @@ -3219,6 +3227,7 @@ async fn upload_with_single_permit() -> Result<(), Box<dyn std::error::Error>> {
Ok(())
}

#[cfg_attr(feature = "nix", ignore)]
#[nativelink_test]
async fn running_actions_manager_respects_action_timeout() -> Result<(), Box<dyn std::error::Error>>
{
Expand Down

0 comments on commit dae772a

Please sign in to comment.