Skip to content

Commit dc08fe7

Browse files
committed
fix(criterion_compat): generate valid URIs when criterion macros are bypassed
When users define a custom main instead of criterion_group!/criterion_main!, current_file and macro_group are never set, producing URIs with empty segments like `::::my_group::my_bench`. Derive current_file from the caller location (track_caller) when the macro did not set it, keeping it inside Criterion, and join URI segments with a shared build_uri helper that skips empty parts so the fork and compat implementations stay in sync.
1 parent 99c7b5a commit dc08fe7

5 files changed

Lines changed: 98 additions & 12 deletions

File tree

crates/codspeed/src/utils.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,37 @@ where
3232
}
3333
}
3434

35+
/// Builds a benchmark URI by joining the non-empty segments with `::`.
36+
///
37+
/// Segments can be empty when `criterion_group!`/`criterion_main!` are bypassed (custom main):
38+
/// `current_file` and `macro_group` are not set in that case and must not produce empty
39+
/// `::` parts (e.g. `::::my_group::my_bench`).
40+
pub fn build_uri(segments: &[&str]) -> String {
41+
segments
42+
.iter()
43+
.filter(|s| !s.is_empty())
44+
.copied()
45+
.collect::<Vec<_>>()
46+
.join("::")
47+
}
48+
49+
/// Resolves the caller's source file path for URI generation, anchored to the
50+
/// workspace root when running under the CodSpeed runner.
51+
///
52+
/// `#[track_caller]` so the location resolves to the original benchmark call site
53+
/// rather than this helper.
54+
#[track_caller]
55+
pub fn caller_file_path() -> String {
56+
let caller = std::panic::Location::caller();
57+
match std::env::var("CODSPEED_CARGO_WORKSPACE_ROOT") {
58+
Ok(workspace_root) => PathBuf::from(workspace_root)
59+
.join(caller.file())
60+
.to_string_lossy()
61+
.into_owned(),
62+
Err(_) => caller.file().to_string(),
63+
}
64+
}
65+
3566
/// Fixes spaces around `::` created by stringify!($function).
3667
pub fn get_formated_function_path(function_path: impl Into<String>) -> String {
3768
let function_path = function_path.into();
@@ -111,6 +142,28 @@ mod tests {
111142
assert_eq!(relative_path, symlink.canonicalize().unwrap());
112143
}
113144

145+
/// COD-2324: empty segments (no file/macro group when `criterion_group!` is bypassed)
146+
/// must not produce URIs like `::::my_group::my_bench`.
147+
#[test]
148+
fn test_build_uri_skips_empty_segments() {
149+
assert_eq!(
150+
build_uri(&["", "", "my_group::my_bench"]),
151+
"my_group::my_bench"
152+
);
153+
assert_eq!(
154+
build_uri(&["benches/custom_main.rs", "", "my_group::my_bench"]),
155+
"benches/custom_main.rs::my_group::my_bench"
156+
);
157+
assert_eq!(
158+
build_uri(&[
159+
"benches/bench.rs",
160+
"benches::bench_fn",
161+
"my_group::my_bench"
162+
]),
163+
"benches/bench.rs::benches::bench_fn::my_group::my_bench"
164+
);
165+
}
166+
114167
#[test]
115168
fn test_get_formated_function_path() {
116169
let input = "std :: vec :: Vec :: new";

crates/criterion_compat/criterion_fork/src/analysis/mod.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -279,12 +279,11 @@ mod codspeed {
279279
}
280280

281281
let git_relative_file_path = codspeed::utils::get_git_relative_path(&c.current_file);
282-
let uri = format!(
283-
"{}::{}::{}",
284-
git_relative_file_path.to_string_lossy(),
282+
let uri = codspeed::utils::build_uri(&[
283+
&git_relative_file_path.to_string_lossy(),
285284
&c.macro_group,
286-
bench_name
287-
);
285+
&bench_name,
286+
]);
288287

289288
(uri, bench_name)
290289
}

crates/criterion_compat/criterion_fork/src/lib.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,18 @@ mod codspeed {
398398
pub fn set_macro_group(&mut self, macro_group: impl Into<String>) {
399399
self.macro_group = macro_group.into();
400400
}
401+
402+
/// Ensures `current_file` is set, for CodSpeed URI generation.
403+
///
404+
/// When set explicitly via `set_current_file`, it is kept as-is; otherwise it is
405+
/// derived from the caller location.
406+
#[track_caller]
407+
pub(crate) fn ensure_current_file(&mut self) {
408+
if !self.current_file.is_empty() {
409+
return;
410+
}
411+
self.current_file = codspeed::utils::caller_file_path();
412+
}
401413
}
402414
}
403415

@@ -1201,7 +1213,9 @@ https://bheisler.github.io/criterion.rs/book/faq.html
12011213
/// ```
12021214
/// # Panics:
12031215
/// Panics if the group name is empty
1216+
#[track_caller]
12041217
pub fn benchmark_group<S: Into<String>>(&mut self, group_name: S) -> BenchmarkGroup<'_, M> {
1218+
self.ensure_current_file();
12051219
let group_name = group_name.into();
12061220
assert!(!group_name.is_empty(), "Group name must not be empty.");
12071221

@@ -1238,6 +1252,7 @@ where
12381252
/// criterion_group!(benches, bench);
12391253
/// criterion_main!(benches);
12401254
/// ```
1255+
#[track_caller]
12411256
pub fn bench_function<F>(&mut self, id: &str, f: F) -> &mut Criterion<M>
12421257
where
12431258
F: FnMut(&mut Bencher<'_, M>),
@@ -1270,6 +1285,7 @@ where
12701285
/// criterion_group!(benches, bench);
12711286
/// criterion_main!(benches);
12721287
/// ```
1288+
#[track_caller]
12731289
pub fn bench_with_input<F, I>(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion<M>
12741290
where
12751291
F: FnMut(&mut Bencher<'_, M>, &I),

crates/criterion_compat/src/compat/criterion.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ impl<M: Measurement> Criterion<M> {
9292
self.macro_group = macro_group.into();
9393
}
9494

95+
#[track_caller]
9596
pub fn bench_function<F>(&mut self, id: &str, f: F) -> &mut Criterion<M>
9697
where
9798
F: FnMut(&mut Bencher),
@@ -101,6 +102,7 @@ impl<M: Measurement> Criterion<M> {
101102
self
102103
}
103104

105+
#[track_caller]
104106
pub fn bench_with_input<F, I>(&mut self, id: BenchmarkId, input: &I, f: F) -> &mut Criterion<M>
105107
where
106108
F: FnMut(&mut Bencher, &I),
@@ -118,9 +120,23 @@ impl<M: Measurement> Criterion<M> {
118120
self
119121
}
120122

123+
#[track_caller]
121124
pub fn benchmark_group<S: Into<String>>(&mut self, group_name: S) -> BenchmarkGroup<M> {
125+
self.ensure_current_file();
122126
BenchmarkGroup::<M>::new(self, group_name.into())
123127
}
128+
129+
/// Ensures `current_file` is set, for CodSpeed URI generation.
130+
///
131+
/// When set explicitly via `set_current_file`, it is kept as-is; otherwise it is
132+
/// derived from the caller location.
133+
#[track_caller]
134+
fn ensure_current_file(&mut self) {
135+
if !self.current_file.is_empty() {
136+
return;
137+
}
138+
self.current_file = codspeed::utils::caller_file_path();
139+
}
124140
}
125141

126142
// Dummy methods

crates/criterion_compat/src/compat/group.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
use std::marker::PhantomData;
22
use std::{cell::RefCell, rc::Rc, time::Duration};
33

4-
use codspeed::{codspeed::CodSpeed, utils::get_git_relative_path};
4+
use codspeed::{
5+
codspeed::CodSpeed,
6+
utils::{build_uri, get_git_relative_path},
7+
};
58
use criterion::measurement::WallTime;
69
use criterion::{measurement::Measurement, PlotConfiguration, SamplingMode, Throughput};
710

@@ -63,12 +66,11 @@ impl<'a, M: Measurement> BenchmarkGroup<'a, M> {
6366
I: ?Sized,
6467
{
6568
let git_relative_file_path = get_git_relative_path(&self.current_file);
66-
let mut uri = format!(
67-
"{}::{}::{}",
68-
git_relative_file_path.to_string_lossy(),
69-
self.macro_group,
70-
self.group_name,
71-
);
69+
let mut uri = build_uri(&[
70+
&git_relative_file_path.to_string_lossy(),
71+
&self.macro_group,
72+
&self.group_name,
73+
]);
7274
if let Some(function_name) = id.function_name {
7375
uri = format!("{uri}::{function_name}");
7476
}

0 commit comments

Comments
 (0)