Update coverage for nativelink-config and nativelink-error

[leodziki]

Description

Increase coverage percentage to 80-100% for nativelink-config and nativelink-error modules

Fixes #1401

Type of change

• New feature (non-breaking change which adds functionality)

Checklist

• Updated documentation if needed
• Tests added/amended

---

This change is 

[aaronmondal]

Thanks a lot for this!

FYI the current CI is failing due to formatting issues. The easiest way to fix formatting is probably by using bazel test ... instead of cargo. You can get the equivalent CI environment locally like so:

• Get a recent version of nix: https://github.com/NixOS/experimental-nix-installer
• In the nativelink repo run nix develop which fetches all the tools at the same versions that CI uses (including bazel, cargo etc)
• Run bazel test //... to invoke the tests via bazel. This should give you a bunch of failures early on so that you don't need to "debug against CI".

Also adding +@allada for review.

Reviewed 5 of 6 files at r1, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained, and 5 of 6 files reviewed, and pending CI: Bazel Dev / macos-13, Bazel Dev / macos-14, Bazel Dev / ubuntu-24.04, NativeLink.com Cloud / Remote Cache / macos-14, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, asan / ubuntu-22.04, docker-compose-compiles-nativelink (22.04), macos-13, pre-commit-checks, ubuntu-22.04, and 7 discussions need to be resolved (waiting on @allada)

---

.gitignore line 26 at r1 (raw file):

nativelink.bazelrc
coverage_report/
.gitignore

We probably shouldn't ignore .gitignore here.

---

nativelink-config/tests/deserialization_test.rs line 1 at r1 (raw file):

use nativelink_config::serde_utils::{

Let's keep the license header.

---

nativelink-config/tests/deserialization_test.rs line 8 at r1 (raw file):

    convert_string_with_shellexpand,
    convert_vec_string_with_shellexpand,
    convert_optional_string_with_shellexpand,

nit: Could we sort these alphabetically?

---

nativelink-config/tests/deserialization_test.rs line 79 at r1 (raw file):

        "#;
    let result: Result<DurationEntity, _> = serde_json5::from_str(example);
    assert!(result.is_err());

Could we make this check more precise by comparing against the expected error message? (here and in all other cases where this currently checks against against .is_err)

---

nativelink-config/tests/deserialization_test.rs line 102 at r1 (raw file):

#[test]
fn test_data_size_invalid_string() {
    let example = r#"

nit: I'm noticing that some of these raw strings have newlines around them and others don't. Is there some reason for this or could we just make all of these consistent? Do we actually need raw strings or would regular strings work as well?

---

nativelink-error/Cargo.toml line 19 at r1 (raw file):

prost = { version = "0.13.3", features = ["std"], default-features = false }
prost-types = { version = "0.13.3", default-features = false }
serde = { version = "1.0", features = ["derive"], default-features = false }

nit: Leave the patch version.

---

nativelink-error/Cargo.toml line 21 at r1 (raw file):

serde = { version = "1.0", features = ["derive"], default-features = false }
tokio = { version = "1.40.0", features = ["fs", "rt-multi-thread", "signal", "io-util"], default-features = false }
tonic = { version = "0.12.3", features = ["transport", "tls", "codegen", "prost"], default-features = false }

nit: Why do we need the codegen and prost features?

[leodziki]

Thanks for the detailed instructions!
I'll follow the instructions on how to test with Bazel and try to fix the formatting issue. Running nix develop seems like a good way to keep your CI and environment in sync.
I'll also double check the review status and see if the discussion with @allada is resolved.
Thanks for the help!
I'll fix the issue as is.

[leodziki]

Including the prost and codegen functions was my choice to simplify code generation and message parsing.
However, it seems unnecessary here.
So I'm going to remove it.
It's also important to note that while consistency in string format can improve code readability and maintainability when switching from raw strings (r#"") to regular strings ("") in your code, the choice between raw strings and regular strings is not always a critical factor. Raw strings are typically used when dealing with complex JSON input that contains escape sequences like \n or ". In the absence of these escape sequences, regular strings can represent the data structure more cleanly and concisely.
I fixed the code by doing everything correctly.
I also added error messages to help identify the cause of the error.