Skip to content

Commit cefba50

Browse files
committed
test: stop using paths from the nginx binary
We had an inconvenient expectation that the nginx binary is installed to the configured location and the default prefix is writable. Instead of that, we should take the same approach as perl tests: * Create a temporary prefix and pass it to nginx. * Use binary from the build dir and allow overriding it with environment variable.
1 parent 713074f commit cefba50

File tree

7 files changed

+82
-35
lines changed

7 files changed

+82
-35
lines changed

Cargo.lock

Lines changed: 16 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,4 @@ vendored = ["nginx-sys/vendored"]
6060
maintenance = { status = "experimental" }
6161

6262
[dev-dependencies]
63-
target-triple = "0.1.2"
63+
tempfile = { version = "3.20.0", default-features = false }

build.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ fn main() {
5353
}
5454
}
5555

56+
// Pass build directory to the tests
57+
println!("cargo::rerun-if-env-changed=DEP_NGINX_BUILD_DIR");
58+
if let Ok(build_dir) = std::env::var("DEP_NGINX_BUILD_DIR") {
59+
println!("cargo::rustc-env=DEP_NGINX_BUILD_DIR={build_dir}");
60+
}
61+
5662
// Generate required compiler flags
5763
if cfg!(target_os = "macos") {
5864
// https://stackoverflow.com/questions/28124221/error-linking-with-cc-failed-exit-code-1

nginx-sys/build/main.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ fn generate_binding(nginx: &NginxSource) {
207207
.map(|path| format!("-I{}", path.to_string_lossy()))
208208
.collect();
209209

210-
print_cargo_metadata(&includes).expect("cargo dependency metadata");
210+
print_cargo_metadata(nginx, &includes).expect("cargo dependency metadata");
211211

212212
// bindgen targets the latest known stable by default
213213
let rust_target: bindgen::RustTarget = env::var("CARGO_PKG_RUST_VERSION")
@@ -294,7 +294,10 @@ fn parse_includes_from_makefile(nginx_autoconf_makefile_path: &PathBuf) -> Vec<P
294294

295295
/// Collect info about the nginx configuration and expose it to the dependents via
296296
/// `DEP_NGINX_...` variables.
297-
pub fn print_cargo_metadata<T: AsRef<Path>>(includes: &[T]) -> Result<(), Box<dyn StdError>> {
297+
pub fn print_cargo_metadata<T: AsRef<Path>>(
298+
nginx: &NginxSource,
299+
includes: &[T],
300+
) -> Result<(), Box<dyn StdError>> {
298301
// Unquote and merge C string constants
299302
let unquote_re = regex::Regex::new(r#""(.*?[^\\])"\s*"#).unwrap();
300303
let unquote = |data: &str| -> String {
@@ -334,6 +337,11 @@ pub fn print_cargo_metadata<T: AsRef<Path>>(includes: &[T]) -> Result<(), Box<dy
334337
}
335338
}
336339

340+
println!(
341+
"cargo::metadata=build_dir={}",
342+
nginx.build_dir.to_str().expect("Unicode build path")
343+
);
344+
337345
println!(
338346
"cargo::metadata=include={}",
339347
// The str conversion is necessary because cargo directives must be valid UTF-8

tests/build.rs

Lines changed: 0 additions & 6 deletions
This file was deleted.

tests/log_test.rs

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
1+
use std::env;
12
use std::fs;
3+
use std::io;
24
use std::io::Result;
35
#[cfg(unix)]
46
use std::os::unix::ffi::OsStrExt;
57
use std::path::{Path, PathBuf};
68
use std::process::Command;
79
use std::process::Output;
810

9-
use ngx::ffi::{NGX_CONF_PATH, NGX_PREFIX, NGX_SBIN_PATH};
11+
const NGINX_BINARY_NAME: &str = "nginx";
1012

1113
/// Convert a CStr to a PathBuf
1214
pub fn cstr_to_path(val: &std::ffi::CStr) -> Option<PathBuf> {
@@ -22,42 +24,68 @@ pub fn cstr_to_path(val: &std::ffi::CStr) -> Option<PathBuf> {
2224
Some(PathBuf::from(str))
2325
}
2426

27+
/// Find nginx binary in the build directory
28+
pub fn find_nginx_binary() -> io::Result<PathBuf> {
29+
let path = [
30+
// TEST_NGINX_BINARY is specified for tests
31+
env::var("TEST_NGINX_BINARY").ok().map(PathBuf::from),
32+
// The module is built against an external NGINX source tree
33+
env::var("NGINX_BUILD_DIR")
34+
.map(PathBuf::from)
35+
.map(|x| x.join(NGINX_BINARY_NAME))
36+
.ok(),
37+
env::var("NGINX_SOURCE_DIR")
38+
.map(PathBuf::from)
39+
.map(|x| x.join("objs").join(NGINX_BINARY_NAME))
40+
.ok(),
41+
// Fallback to the build directory exposed by nginx-sys
42+
option_env!("DEP_NGINX_BUILD_DIR")
43+
.map(PathBuf::from)
44+
.map(|x| x.join(NGINX_BINARY_NAME)),
45+
]
46+
.into_iter()
47+
.flatten()
48+
.find(|x| x.is_file())
49+
.ok_or(io::ErrorKind::NotFound)?;
50+
51+
Ok(path)
52+
}
53+
2554
/// harness to test nginx
2655
pub struct Nginx {
27-
pub install_path: PathBuf,
56+
pub prefix: tempfile::TempDir,
57+
pub bin_path: PathBuf,
2858
pub config_path: PathBuf,
2959
}
3060

3161
impl Default for Nginx {
3262
/// create nginx with default
3363
fn default() -> Nginx {
34-
let install_path = cstr_to_path(NGX_PREFIX).expect("installation prefix");
35-
Nginx::new(install_path)
64+
let binary = find_nginx_binary().expect("nginx binary");
65+
Nginx::new(binary).expect("test harness")
3666
}
3767
}
3868

3969
impl Nginx {
40-
pub fn new<P: AsRef<Path>>(path: P) -> Nginx {
41-
let install_path = path.as_ref();
42-
let config_path = cstr_to_path(NGX_CONF_PATH).expect("configuration path");
43-
let config_path = install_path.join(config_path);
44-
45-
Nginx {
46-
install_path: install_path.into(),
47-
config_path,
48-
}
49-
}
70+
pub fn new(binary: impl AsRef<Path>) -> io::Result<Nginx> {
71+
let prefix = tempfile::tempdir()?;
72+
let config = prefix.path().join("nginx.conf");
73+
74+
fs::create_dir(prefix.path().join("logs"))?;
5075

51-
/// get bin path to nginx instance
52-
pub fn bin_path(&mut self) -> PathBuf {
53-
let bin_path = cstr_to_path(NGX_SBIN_PATH).expect("binary path");
54-
self.install_path.join(bin_path)
76+
Ok(Nginx {
77+
prefix,
78+
bin_path: binary.as_ref().to_owned(),
79+
config_path: config,
80+
})
5581
}
5682

5783
/// start nginx process with arguments
58-
pub fn cmd(&mut self, args: &[&str]) -> Result<Output> {
59-
let bin_path = self.bin_path();
60-
let result = Command::new(bin_path).args(args).output();
84+
pub fn cmd(&self, args: &[&str]) -> Result<Output> {
85+
let prefix = self.prefix.path().to_string_lossy();
86+
let config_path = self.config_path.to_string_lossy();
87+
let args = [&["-p", &prefix, "-c", &config_path], args].concat();
88+
let result = Command::new(&self.bin_path).args(args).output();
6189

6290
match result {
6391
Err(e) => Err(e),

tests/nginx.conf

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ events {
1616

1717

1818
http {
19-
include mime.types;
2019
default_type application/octet-stream;
2120

2221

0 commit comments

Comments
 (0)