chore(cli): rename --trace-ops to --trace-leaks (#22598)

As we add tracing to more types of runtime activity, `--trace-ops` is
less useful of a name. `--trace-leaks` better reflects that this feature
traces both ops and timers, and will eventually trace resource opening
as well.

This keeps `--trace-ops` as an alias for `--trace-leaks`, but prints a
warning to the console suggesting migration to `--trace-leaks`.

One test continues to use `--trace-ops` to test the deprecation warning.

---------

Signed-off-by: Matt Mastracci <matthew@mastracci.com>
This commit is contained in:
Matt Mastracci 2024-02-28 09:12:43 -07:00 committed by GitHub
parent c2c4e745a5
commit a3f982c1d5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 51 additions and 31 deletions

View file

@ -275,7 +275,7 @@ pub struct TestFlags {
pub filter: Option<String>,
pub shuffle: Option<u64>,
pub concurrent_jobs: Option<NonZeroUsize>,
pub trace_ops: bool,
pub trace_leaks: bool,
pub watch: Option<WatchFlags>,
pub reporter: TestReporterConfig,
pub junit_path: Option<String>,
@ -2155,7 +2155,14 @@ Directory arguments are expanded to all contained files matching the glob
.arg(
Arg::new("trace-ops")
.long("trace-ops")
.help("Enable tracing of async ops. Useful when debugging leaking ops in test, but impacts test execution time.")
.help("Deprecated alias for --trace-leaks.")
.hide(true)
.action(ArgAction::SetTrue),
)
.arg(
Arg::new("trace-leaks")
.long("trace-leaks")
.help("Enable tracing of leaks. Useful when debugging leaking ops in test, but impacts test execution time.")
.action(ArgAction::SetTrue),
)
.arg(
@ -3704,7 +3711,18 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) {
};
let no_run = matches.get_flag("no-run");
let trace_ops = matches.get_flag("trace-ops");
let trace_leaks =
matches.get_flag("trace-ops") || matches.get_flag("trace-leaks");
if trace_leaks && matches.get_flag("trace-ops") {
// We can't change this to use the log crate because its not configured
// yet at this point since the flags haven't been parsed. This flag is
// deprecated though so it's not worth changing the code to use the log
// crate here and this is only done for testing anyway.
eprintln!(
"⚠️ {}",
crate::colors::yellow("The `--trace-ops` flag is deprecated and will be removed in Deno 2.0.\nUse the `--trace-leaks` flag instead."),
);
}
let doc = matches.get_flag("doc");
let allow_none = matches.get_flag("allow-none");
let filter = matches.remove_one::<String>("filter");
@ -3792,7 +3810,7 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) {
shuffle,
allow_none,
concurrent_jobs,
trace_ops,
trace_leaks,
watch: watch_arg_parse(matches),
reporter,
junit_path,
@ -7098,7 +7116,7 @@ mod tests {
#[test]
fn test_with_flags() {
#[rustfmt::skip]
let r = flags_from_vec(svec!["deno", "test", "--unstable", "--no-npm", "--no-remote", "--trace-ops", "--no-run", "--filter", "- foo", "--coverage=cov", "--location", "https:foo", "--allow-net", "--allow-none", "dir1/", "dir2/", "--", "arg1", "arg2"]);
let r = flags_from_vec(svec!["deno", "test", "--unstable", "--no-npm", "--no-remote", "--trace-leaks", "--no-run", "--filter", "- foo", "--coverage=cov", "--location", "https:foo", "--allow-net", "--allow-none", "dir1/", "dir2/", "--", "arg1", "arg2"]);
assert_eq!(
r.unwrap(),
Flags {
@ -7114,7 +7132,7 @@ mod tests {
},
shuffle: None,
concurrent_jobs: None,
trace_ops: true,
trace_leaks: true,
coverage_dir: Some("cov".to_string()),
watch: Default::default(),
reporter: Default::default(),
@ -7196,7 +7214,7 @@ mod tests {
ignore: vec![],
},
concurrent_jobs: Some(NonZeroUsize::new(4).unwrap()),
trace_ops: false,
trace_leaks: false,
coverage_dir: None,
watch: Default::default(),
junit_path: None,
@ -7229,7 +7247,7 @@ mod tests {
ignore: vec![],
},
concurrent_jobs: None,
trace_ops: false,
trace_leaks: false,
coverage_dir: None,
watch: Default::default(),
reporter: Default::default(),
@ -7267,7 +7285,7 @@ mod tests {
ignore: vec![],
},
concurrent_jobs: None,
trace_ops: false,
trace_leaks: false,
coverage_dir: None,
watch: Default::default(),
reporter: Default::default(),
@ -7384,7 +7402,7 @@ mod tests {
ignore: vec![],
},
concurrent_jobs: None,
trace_ops: false,
trace_leaks: false,
coverage_dir: None,
watch: Default::default(),
reporter: Default::default(),
@ -7415,7 +7433,7 @@ mod tests {
ignore: vec![],
},
concurrent_jobs: None,
trace_ops: false,
trace_leaks: false,
coverage_dir: None,
watch: Some(Default::default()),
reporter: Default::default(),
@ -7445,7 +7463,7 @@ mod tests {
ignore: vec![],
},
concurrent_jobs: None,
trace_ops: false,
trace_leaks: false,
coverage_dir: None,
watch: Some(Default::default()),
reporter: Default::default(),
@ -7477,7 +7495,7 @@ mod tests {
ignore: vec![],
},
concurrent_jobs: None,
trace_ops: false,
trace_leaks: false,
coverage_dir: None,
watch: Some(WatchFlags {
hmr: false,

View file

@ -333,7 +333,7 @@ pub struct TestOptions {
pub filter: Option<String>,
pub shuffle: Option<u64>,
pub concurrent_jobs: NonZeroUsize,
pub trace_ops: bool,
pub trace_leaks: bool,
pub reporter: TestReporterConfig,
pub junit_path: Option<String>,
}
@ -361,7 +361,7 @@ impl TestOptions {
filter: test_flags.filter,
no_run: test_flags.no_run,
shuffle: test_flags.shuffle,
trace_ops: test_flags.trace_ops,
trace_leaks: test_flags.trace_leaks,
reporter: test_flags.reporter,
junit_path: test_flags.junit_path,
})

View file

@ -296,7 +296,7 @@ impl TestRun {
test::TestSpecifierOptions {
filter,
shuffle: None,
trace_ops: false,
trace_leaks: false,
},
))
}
@ -441,7 +441,7 @@ impl TestRun {
.iter()
.map(|s| s.as_str()),
);
args.push("--trace-ops");
args.push("--trace-leaks");
if self.workspace_settings.unstable && !args.contains(&"--unstable") {
args.push("--unstable");
}

View file

@ -107,7 +107,7 @@ fn format_sanitizer_accum(
}
let mut output = vec![];
let mut needs_trace_ops = false;
let mut needs_trace_leaks = false;
for ((item_type, item_name, trace), count) in accum.into_iter() {
if item_type == RuntimeActivityType::Resource {
let (name, action1, action2) = pretty_resource_name(&item_name);
@ -143,7 +143,7 @@ fn format_sanitizer_accum(
value += &if let Some(trace) = trace {
format!(" The operation {tense} started here:\n{trace}")
} else {
needs_trace_ops = true;
needs_trace_leaks = true;
String::new()
};
output.push(value);
@ -157,8 +157,8 @@ fn format_sanitizer_accum(
);
}
}
if needs_trace_ops {
(output, vec!["To get more details where ops were leaked, run again with --trace-ops flag.".to_owned()])
if needs_trace_leaks {
(output, vec!["To get more details where leaks occurred, run again with the --trace-leaks flag.".to_owned()])
} else {
(output, vec![])
}
@ -360,5 +360,5 @@ mod tests {
// https://github.com/denoland/deno/issues/13938
leak_format_test!(op_unknown, true, [RuntimeActivity::AsyncOp(0, None, "op_unknown")],
" - An async call to op_unknown was started in this test, but never completed.\n\
To get more details where ops were leaked, run again with --trace-ops flag.\n");
To get more details where leaks occurred, run again with the --trace-leaks flag.\n");
}

View file

@ -440,7 +440,7 @@ struct TestSpecifiersOptions {
pub struct TestSpecifierOptions {
pub shuffle: Option<u64>,
pub filter: TestFilter,
pub trace_ops: bool,
pub trace_leaks: bool,
}
impl TestSummary {
@ -559,7 +559,7 @@ async fn test_specifier_inner(
let mut coverage_collector = worker.maybe_setup_coverage_collector().await?;
if options.trace_ops {
if options.trace_leaks {
worker.execute_script_static(
located_script_name!(),
"Deno[Deno.internal].core.setLeakTracingEnabled(true);",
@ -1503,7 +1503,7 @@ pub async fn run_tests(
specifier: TestSpecifierOptions {
filter: TestFilter::from_flag(&test_options.filter),
shuffle: test_options.shuffle,
trace_ops: test_options.trace_ops,
trace_leaks: test_options.trace_leaks,
},
},
)
@ -1647,7 +1647,7 @@ pub async fn run_tests_with_watch(
specifier: TestSpecifierOptions {
filter: TestFilter::from_flag(&test_options.filter),
shuffle: test_options.shuffle,
trace_ops: test_options.trace_ops,
trace_leaks: test_options.trace_leaks,
},
},
)

View file

@ -231,7 +231,7 @@ itest!(ops_sanitizer_timeout_failure {
itest!(ops_sanitizer_multiple_timeout_tests {
args:
"test --trace-ops test/sanitizer/ops_sanitizer_multiple_timeout_tests.ts",
"test --trace-leaks test/sanitizer/ops_sanitizer_multiple_timeout_tests.ts",
exit_code: 1,
output: "test/sanitizer/ops_sanitizer_multiple_timeout_tests.out",
});
@ -243,13 +243,13 @@ itest!(ops_sanitizer_multiple_timeout_tests_no_trace {
});
itest!(sanitizer_trace_ops_catch_error {
args: "test -A --trace-ops test/sanitizer/trace_ops_caught_error/main.ts",
args: "test -A --trace-leaks test/sanitizer/trace_ops_caught_error/main.ts",
exit_code: 0,
output: "test/sanitizer/trace_ops_caught_error/main.out",
});
itest!(ops_sanitizer_closed_inside_started_before {
args: "test --trace-ops test/sanitizer/ops_sanitizer_closed_inside_started_before.ts",
args: "test --trace-leaks test/sanitizer/ops_sanitizer_closed_inside_started_before.ts",
exit_code: 1,
output: "test/sanitizer/ops_sanitizer_closed_inside_started_before.out",
});

View file

@ -8,12 +8,12 @@ test 2 ... FAILED ([WILDCARD])
test 1 => [WILDCARD]/ops_sanitizer_multiple_timeout_tests.ts:[WILDCARD]
error: Leaks detected:
- 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call.
To get more details where ops were leaked, run again with --trace-ops flag.
To get more details where leaks occurred, run again with the --trace-leaks flag.
test 2 => [WILDCARD]/ops_sanitizer_multiple_timeout_tests.ts:[WILDCARD]
error: Leaks detected:
- 2 async operations to sleep for a duration were started in this test, but never completed. This is often caused by not cancelling a `setTimeout` or `setInterval` call.
To get more details where ops were leaked, run again with --trace-ops flag.
To get more details where leaks occurred, run again with the --trace-leaks flag.
FAILURES

View file

@ -1,3 +1,5 @@
⚠️ The `--trace-ops` flag is deprecated and will be removed in Deno 2.0.
Use the `--trace-leaks` flag instead.
Check [WILDCARD]/ops_sanitizer_unstable.ts
running 2 tests from [WILDCARD]/ops_sanitizer_unstable.ts
no-op ... ok ([WILDCARD])