Skip to content

fix: propagate errors #47

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

Merged
merged 21 commits into from
Sep 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
File renamed without changes.
31 changes: 31 additions & 0 deletions .config/nextest.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# required minimum nextest version
nextest-version = "0.9.77"

[profile.default]
# A profile to run most tests, except tests that run longer than 10 seconds
default-filter = "not test(#*rate_limit_secondary) - test(#git::test::with_*)"

# This will flag any test that runs longer than 10 seconds. Useful when writing new tests.
slow-timeout = "10s"

[profile.ci]
# A profile to run only tests that use clang-tidy and/or clang-format
# NOTE: This profile is intended to keep CI runtime low. Locally, use default or all profiles

# This is all tests in tests/ folder + unit test for --extra-args.
default-filter = "kind(test) + test(#*use_extra_args)"

# show wich tests were skipped
status-level = "skip"

# show log output from each test
success-output = "final"
failure-output = "immediate-final"

[profile.all]
# A profile to run all tests (including tests that run longer than 10 seconds)
default-filter = "all()"

# Revert slow-timeout to nextest default value.
# Otherwise, default profile value (10s) is inherited.
slow-timeout = "60s"
2 changes: 1 addition & 1 deletion .github/workflows/bump-n-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
uses: orhun/git-cliff-action@v4
id: git-cliff
with:
config: cliff.toml
config: .config/cliff.toml
args: --unreleased
env:
OUTPUT: ${{ runner.temp }}/changes.md
Expand Down
11 changes: 10 additions & 1 deletion .github/workflows/bump_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,16 @@ def main():
subprocess.run(["napi", "version"], cwd="node-binding", check=True)
print("Updated version in node-binding/**package.json")

subprocess.run(["git", "cliff", "--tag", Updater.new_version], check=True)
subprocess.run(
[
"git-cliff",
"--config",
".config/cliff.toml",
"--tag",
Updater.new_version,
],
check=True,
)
print("Updated CHANGELOG.md")

subprocess.run(["git", "add", "--all"], check=True)
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/python-packaging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: 3.x
- name: Build sdist
uses: PyO3/maturin-action@v1
with:
Expand Down
26 changes: 12 additions & 14 deletions .github/workflows/run-dev-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ jobs:
if: runner.os == 'Linux'
env:
CLANG_VERSION: '7'
run: just test
run: just test ci

- name: Install clang v8
if: runner.os == 'Linux'
Expand All @@ -100,7 +100,7 @@ jobs:
if: runner.os == 'Linux'
env:
CLANG_VERSION: '8'
run: just test
run: just test ci

- name: Install clang v9
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -110,7 +110,7 @@ jobs:
- name: Collect Coverage for clang v9
env:
CLANG_VERSION: '9'
run: just test
run: just test ci

- name: Install clang v10
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -120,7 +120,7 @@ jobs:
- name: Collect Coverage for clang v10
env:
CLANG_VERSION: '10'
run: just test
run: just test ci

- name: Install clang 11
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -130,7 +130,7 @@ jobs:
- name: Collect Coverage for clang v11
env:
CLANG_VERSION: '11'
run: just test
run: just test ci

- name: Install clang 12
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -140,7 +140,7 @@ jobs:
- name: Collect Coverage for clang v12
env:
CLANG_VERSION: '12'
run: just test
run: just test ci

- name: Install clang 13
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -150,7 +150,7 @@ jobs:
- name: Collect Coverage for clang v13
env:
CLANG_VERSION: '13'
run: just test
run: just test ci

- name: Install clang 14
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -160,7 +160,7 @@ jobs:
- name: Collect Coverage for clang v14
env:
CLANG_VERSION: '14'
run: just test
run: just test ci

- name: Install clang 15
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -170,7 +170,7 @@ jobs:
- name: Collect Coverage for clang v15
env:
CLANG_VERSION: '15'
run: just test
run: just test ci

- name: Install clang 16
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -180,7 +180,7 @@ jobs:
- name: Collect Coverage for clang v16
env:
CLANG_VERSION: '16'
run: just test
run: just test ci

- name: Install clang 17
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -190,7 +190,7 @@ jobs:
- name: Collect Coverage for clang v17
env:
CLANG_VERSION: '17'
run: just test
run: just test ci

- name: Install clang 18
uses: cpp-linter/cpp_linter_rs/install-clang-action@main
Expand All @@ -200,7 +200,7 @@ jobs:
- name: Collect Coverage for clang v18
env:
CLANG_VERSION: '18'
run: just test --run-ignored=all
run: just test all

- name: Generate Coverage HTML report
run: just pretty-cov
Expand All @@ -212,13 +212,11 @@ jobs:
path: target/llvm-cov-pretty

- name: Generate Coverage lcov report
if: runner.os == 'Linux'
run: |
rm coverage.json
just lcov

- uses: codecov/codecov-action@v4
if: runner.os == 'Linux'
with:
token: ${{secrets.CODECOV_TOKEN}}
files: lcov.info
Expand Down
2 changes: 2 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions cpp-linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ license.workspace = true
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
anyhow = "1.0.89"
chrono = "0.4.38"
clap = "4.5.17"
fast-glob = "0.4.0"
Expand Down
56 changes: 29 additions & 27 deletions cpp-linter/src/clang_tools/clang_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
sync::{Arc, Mutex, MutexGuard},
};

use anyhow::{Context, Result};
use log::Level;
// non-std crates
use serde::Deserialize;
Expand Down Expand Up @@ -56,13 +57,15 @@
///
/// This value is not provided by the XML output, but we calculate it after
/// deserialization.
pub line: Option<usize>,
#[serde(default)]
pub line: usize,

/// The column number on the line described by the [`Replacement::offset`].
///
/// This value is not provided by the XML output, but we calculate it after
/// deserialization.
pub cols: Option<usize>,
#[serde(default)]
pub cols: usize,
}

impl Clone for Replacement {
Expand Down Expand Up @@ -109,15 +112,16 @@
pub fn run_clang_format(
file: &mut MutexGuard<FileObj>,
clang_params: &ClangParams,
) -> Vec<(log::Level, String)> {
) -> Result<Vec<(log::Level, String)>> {
let mut cmd = Command::new(clang_params.clang_format_command.as_ref().unwrap());
let mut logs = vec![];
cmd.args(["--style", &clang_params.style]);
let ranges = file.get_ranges(&clang_params.lines_changed_only);
for range in &ranges {
cmd.arg(format!("--lines={}:{}", range.start(), range.end()));
}
cmd.arg(file.name.to_string_lossy().as_ref());
let file_name = file.name.to_string_lossy().to_string();
cmd.arg(file.name.to_path_buf().as_os_str());
let mut patched = None;
if clang_params.format_review {
logs.push((
Expand All @@ -129,7 +133,7 @@
.as_ref()
.unwrap()
.to_str()
.unwrap(),
.unwrap_or_default(),
cmd.get_args()
.map(|a| a.to_str().unwrap())
.collect::<Vec<&str>>()
Expand All @@ -138,7 +142,7 @@
));
patched = Some(
cmd.output()
.expect("Failed to get fixes from clang-format")
.with_context(|| format!("Failed to get fixes from clang-format: {file_name}"))?
.stdout,
);
}
Expand All @@ -147,33 +151,30 @@
log::Level::Info,
format!(
"Running \"{} {}\"",
clang_params
.clang_format_command
.as_ref()
.unwrap()
.to_str()
.unwrap(),
cmd.get_program().to_string_lossy(),
cmd.get_args()
.map(|x| x.to_str().unwrap())
.collect::<Vec<&str>>()
.join(" ")
),
));
let output = cmd.output().unwrap();
let output = cmd
.output()
.with_context(|| format!("Failed to get replacements from clang-format: {file_name}"))?;
if !output.stderr.is_empty() || !output.status.success() {
logs.push((
log::Level::Debug,
format!(
"clang-format raised the follow errors:\n{}",
String::from_utf8(output.stderr).unwrap()
String::from_utf8_lossy(&output.stderr)

Check warning on line 169 in cpp-linter/src/clang_tools/clang_format.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter/src/clang_tools/clang_format.rs#L169

Added line #L169 was not covered by tests
),
));
}
if output.stdout.is_empty() {
return logs;
return Ok(logs);

Check warning on line 174 in cpp-linter/src/clang_tools/clang_format.rs

View check run for this annotation

Codecov / codecov/patch

cpp-linter/src/clang_tools/clang_format.rs#L174

Added line #L174 was not covered by tests
}
let xml = String::from_utf8(output.stdout)
.unwrap()
.with_context(|| format!("stdout from clang-format was not UTF-8 encoded: {file_name}"))?
.lines()
.collect::<Vec<&str>>()
.join("");
Expand All @@ -189,11 +190,12 @@
});
format_advice.patched = patched;
if !format_advice.replacements.is_empty() {
// get line and column numbers from format_advice.offset
let mut filtered_replacements = Vec::new();
for replacement in &mut format_advice.replacements {
let (line_number, columns) = get_line_cols_from_offset(&file.name, replacement.offset);
replacement.line = Some(line_number);
replacement.cols = Some(columns);
replacement.line = line_number;
replacement.cols = columns;
for range in &ranges {
if range.contains(&line_number.try_into().unwrap_or(0)) {
filtered_replacements.push(replacement.clone());
Expand All @@ -208,7 +210,7 @@
format_advice.replacements = filtered_replacements;
}
file.format_advice = Some(format_advice);
logs
Ok(logs)
}

#[cfg(test)]
Expand All @@ -234,29 +236,29 @@
offset: 113,
length: 5,
value: Some(String::from("\n ")),
line: None,
cols: None,
line: 0,
cols: 0,
},
Replacement {
offset: 147,
length: 0,
value: Some(String::from(" ")),
line: None,
cols: None,
line: 0,
cols: 0,
},
Replacement {
offset: 161,
length: 0,
value: None,
line: None,
cols: None,
line: 0,
cols: 0,
},
Replacement {
offset: 165,
length: 19,
value: Some(String::from("\n\n")),
line: None,
cols: None,
line: 0,
cols: 0,
},
],
patched: None,
Expand Down
Loading
Loading