Skip to content

Commit bb073fd

Browse files
snthCopilot
andcommitted
fix: resolve CI failures for dead code and binary discovery
- Remove unreachable maybe_strip_colors #[cfg(not(feature = "display"))] variant that caused dead_code error on wasm32 builds - Add shared test_utils module with prqlc_command() that auto-builds the prqlc binary on demand using std::sync::Once, fixing test failures when cargo test doesn't build the binary (e.g. minimal-versions check) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1bafee3 commit bb073fd

6 files changed

Lines changed: 71 additions & 83 deletions

File tree

prqlc/prqlc-cli/src/cli/docs_generator.rs

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -310,10 +310,9 @@ Generated with [prqlc](https://prql-lang.org/) {}.
310310

311311
#[cfg(test)]
312312
mod tests {
313-
use std::process::Command;
314-
315313
use insta_cmd::assert_cmd_snapshot;
316-
use insta_cmd::get_cargo_bin;
314+
315+
use super::super::test_utils::prqlc_command;
317316

318317
#[test]
319318
fn generate_html_docs() {
@@ -508,23 +507,4 @@ mod tests {
508507
----- stderr -----
509508
");
510509
}
511-
512-
fn prqlc_command() -> Command {
513-
let mut cmd = Command::new(get_cargo_bin("prqlc"));
514-
normalize_prqlc(&mut cmd);
515-
cmd
516-
}
517-
518-
fn normalize_prqlc(cmd: &mut Command) -> &mut Command {
519-
cmd
520-
// We set `CLICOLOR_FORCE` in CI to force color output, but we don't want `prqlc` to
521-
// output color for our snapshot tests. And it seems to override the
522-
// `--color=never` flag.
523-
.env_remove("CLICOLOR_FORCE")
524-
.env("NO_COLOR", "1")
525-
.args(["--color=never"])
526-
// We don't want the tests to be affected by the user's `RUST_BACKTRACE` setting.
527-
.env_remove("RUST_BACKTRACE")
528-
.env_remove("RUST_LOG")
529-
}
530510
}

prqlc/prqlc-cli/src/cli/highlight.rs

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,9 @@ fn is_transform(ident: &str) -> bool {
103103

104104
#[cfg(test)]
105105
mod tests {
106-
use std::process::Command;
107-
108106
use insta_cmd::assert_cmd_snapshot;
109-
use insta_cmd::get_cargo_bin;
107+
108+
use super::super::test_utils::prqlc_command;
110109

111110
#[test]
112111
fn highlight() {
@@ -138,24 +137,4 @@ mod tests {
138137
----- stderr -----
139138
"#);
140139
}
141-
142-
// TODO: import from existing location, need to adjust visibility
143-
fn prqlc_command() -> Command {
144-
let mut cmd = Command::new(get_cargo_bin("prqlc"));
145-
normalize_prqlc(&mut cmd);
146-
cmd
147-
}
148-
149-
fn normalize_prqlc(cmd: &mut Command) -> &mut Command {
150-
cmd
151-
// We set `CLICOLOR_FORCE` in CI to force color output, but we don't want `prqlc` to
152-
// output color for our snapshot tests. And it seems to override the
153-
// `--color=never` flag.
154-
.env_remove("CLICOLOR_FORCE")
155-
.env("NO_COLOR", "1")
156-
.args(["--color=never"])
157-
// We don't want the tests to be affected by the user's `RUST_BACKTRACE` setting.
158-
.env_remove("RUST_BACKTRACE")
159-
.env_remove("RUST_LOG")
160-
}
161140
}

prqlc/prqlc-cli/src/cli/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ mod jinja;
3737
mod lsp;
3838
#[cfg(test)]
3939
mod test;
40+
#[cfg(test)]
41+
mod test_utils;
4042
mod watch;
4143

4244
/// CLI entrypoint

prqlc/prqlc-cli/src/cli/test.rs

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@ use std::env::current_dir;
22
use std::fs;
33
use std::path::Path;
44
use std::path::PathBuf;
5-
use std::process::Command;
65
use std::str::FromStr;
76

87
use insta_cmd::assert_cmd_snapshot;
98
use tempfile::TempDir;
109
use walkdir::WalkDir;
1110

11+
use super::test_utils::prqlc_command;
12+
1213
#[cfg(not(windows))] // Windows has slightly different output (e.g. `prqlc.exe`), so we exclude.
1314
#[test]
1415
fn help() {
@@ -581,37 +582,6 @@ fn project_path() -> PathBuf {
581582
.join("../prqlc/tests/integration/project")
582583
}
583584

584-
fn prqlc_command() -> Command {
585-
let bin = std::env::var_os("CARGO_BIN_EXE_prqlc")
586-
.map(PathBuf::from)
587-
.unwrap_or_else(|| {
588-
// CARGO_BIN_EXE_* is only set for integration tests. For unit tests
589-
// (including nextest), find the binary relative to the test executable.
590-
let test_bin = std::env::current_exe().expect("cannot determine test binary path");
591-
let mut dir = test_bin.parent().unwrap().to_path_buf();
592-
if dir.ends_with("deps") {
593-
dir.pop();
594-
}
595-
dir.join("prqlc")
596-
});
597-
let mut cmd = Command::new(bin);
598-
normalize_prqlc(&mut cmd);
599-
cmd
600-
}
601-
602-
fn normalize_prqlc(cmd: &mut Command) -> &mut Command {
603-
cmd
604-
// We set `CLICOLOR_FORCE` in CI to force color output, but we don't want `prqlc` to
605-
// output color for our snapshot tests. And it seems to override the
606-
// `--color=never` flag.
607-
.env_remove("CLICOLOR_FORCE")
608-
.env("NO_COLOR", "1")
609-
.args(["--color=never"])
610-
// We don't want the tests to be affected by the user's `RUST_BACKTRACE` setting.
611-
.env_remove("RUST_BACKTRACE")
612-
.env_remove("RUST_LOG")
613-
}
614-
615585
#[test]
616586
fn compile_no_prql_files() {
617587
assert_cmd_snapshot!(prqlc_command().args(["compile", "Cargo.toml"]), @r"
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
use std::path::PathBuf;
2+
use std::process::Command;
3+
use std::sync::Once;
4+
5+
static BUILD_PRQLC: Once = Once::new();
6+
7+
/// Return a `Command` that runs the `prqlc` binary with color/backtrace
8+
/// stripped so snapshot tests are deterministic.
9+
///
10+
/// When `CARGO_BIN_EXE_prqlc` is set (integration tests), it uses that path.
11+
/// Otherwise it locates the binary relative to the test executable and builds
12+
/// it if necessary — `cargo test` for a bin-only crate does not produce the
13+
/// non-test binary automatically.
14+
pub fn prqlc_command() -> Command {
15+
let bin = prqlc_bin_path();
16+
let mut cmd = Command::new(bin);
17+
normalize_prqlc(&mut cmd);
18+
cmd
19+
}
20+
21+
fn prqlc_bin_path() -> PathBuf {
22+
if let Some(bin) = std::env::var_os("CARGO_BIN_EXE_prqlc") {
23+
return PathBuf::from(bin);
24+
}
25+
26+
// Locate the target directory from the test binary path.
27+
let test_bin = std::env::current_exe().expect("cannot determine test binary path");
28+
let mut dir = test_bin.parent().unwrap().to_path_buf();
29+
if dir.ends_with("deps") {
30+
dir.pop();
31+
}
32+
33+
let bin_name = if cfg!(windows) { "prqlc.exe" } else { "prqlc" };
34+
let bin_path = dir.join(bin_name);
35+
36+
// `cargo test` for a [[bin]]-only crate builds the test harness (in deps/)
37+
// but does NOT build the actual binary. Build it on demand if missing.
38+
if !bin_path.exists() {
39+
BUILD_PRQLC.call_once(|| {
40+
let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_string());
41+
let status = Command::new(cargo)
42+
.args(["build", "--bin", "prqlc"])
43+
.status()
44+
.expect("failed to run `cargo build --bin prqlc`");
45+
assert!(status.success(), "failed to build prqlc binary");
46+
});
47+
}
48+
49+
bin_path
50+
}
51+
52+
fn normalize_prqlc(cmd: &mut Command) -> &mut Command {
53+
cmd
54+
// We set `CLICOLOR_FORCE` in CI to force color output, but we don't want `prqlc` to
55+
// output color for our snapshot tests. And it seems to override the
56+
// `--color=never` flag.
57+
.env_remove("CLICOLOR_FORCE")
58+
.env("NO_COLOR", "1")
59+
.args(["--color=never"])
60+
// We don't want the tests to be affected by the user's `RUST_BACKTRACE` setting.
61+
.env_remove("RUST_BACKTRACE")
62+
.env_remove("RUST_LOG")
63+
}

prqlc/prqlc/src/utils/mod.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,6 @@ pub fn maybe_strip_colors(s: &str) -> String {
129129
}
130130
}
131131

132-
/// When the `display` feature is disabled, return the string unchanged.
133-
#[cfg(not(feature = "display"))]
134-
pub fn maybe_strip_colors(s: &str) -> String {
135-
s.to_string()
136-
}
137-
138132
#[test]
139133
fn test_write_ident_part() {
140134
assert!(!valid_ident().is_match(""));

0 commit comments

Comments
 (0)