-
Notifications
You must be signed in to change notification settings - Fork 122
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
Introduce reproducible branch-based coverage #1375
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+@adam-singer +@allada +@blakehatch +@caass +@SchahinRohani
Reviewable status: 0 of 5 LGTMs obtained, and 0 of 8 files reviewed, and pending CI: Bazel Dev / macos-13, Bazel Dev / macos-14, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-13, Cargo Dev / ubuntu-22.04, Installation / macos-13, Installation / macos-14, Installation / ubuntu-22.04, NativeLink.com Cloud / Remote Cache / macos-14, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / large-ubuntu-22.04, Web Platform Deployment / macos-14, Web Platform Deployment / ubuntu-24.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (22.04), integration-tests (22.04), macos-13, ubuntu-20.04 / stable, ubuntu-22.04, ubuntu-22.04 / stable, windows-2022 / stable (waiting on @adam-singer, @allada, @blakehatch, @caass, and @SchahinRohani)
dae772a
to
bf29068
Compare
Preview at https://aaronmondal.github.io/nativelink/ |
bf29068
to
329889d
Compare
329889d
to
f3c8caf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 9 of 10 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 1 of 5 LGTMs obtained, and all files reviewed, and 1 discussions need to be resolved (waiting on @allada, @blakehatch, @caass, and @SchahinRohani)
.github/workflows/coverage.yaml
line 24 at r2 (raw file):
- name: Checkout uses: >- # v4.1.1 actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
yaml / github workflows can we do a single definition of uses
value statement? I know yaml supports inheritance, question I guess would be if github workflow yaml supports that
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. Pushes to main publish the site at tracemachina.github.io/nativelink. 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-@allada -@blakehatch -@caass -@SchahinRohani
Reviewed 5 of 10 files at r1, 1 of 1 files at r2, 4 of 4 files at r3, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and all files reviewed, and pending CI: Bazel Dev / macos-13, Coverage, Installation / macos-13, Remote / large-ubuntu-22.04
.github/workflows/coverage.yaml
line 24 at r2 (raw file):
Previously, adam-singer (Adam Singer) wrote…
yaml / github workflows can we do a single definition of
uses
value statement? I know yaml supports inheritance, question I guess would be if github workflow yaml supports that
Reports may now be build via:
The
result
symlink then contains the contents of a webpage to view theexisting reports.
Pushes to main publish the site at tracemachina.github.io/nativelink.
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 notideal, 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.
This change is