Skip to content

Commit 6e23959

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 c94b78c commit 6e23959

File tree

7 files changed

+83
-35
lines changed

7 files changed

+83
-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
@@ -45,4 +45,4 @@ vendored = ["nginx-sys/vendored"]
4545
maintenance = { status = "experimental" }
4646

4747
[dev-dependencies]
48-
target-triple = "0.1.2"
48+
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
@@ -202,7 +202,7 @@ fn generate_binding(nginx: &NginxSource) {
202202
.map(|path| format!("-I{}", path.to_string_lossy()))
203203
.collect();
204204

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

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

288288
/// Collect info about the nginx configuration and expose it to the dependents via
289289
/// `DEP_NGINX_...` variables.
290-
pub fn print_cargo_metadata<T: AsRef<Path>>(includes: &[T]) -> Result<(), Box<dyn StdError>> {
290+
pub fn print_cargo_metadata<T: AsRef<Path>>(
291+
nginx: &NginxSource,
292+
includes: &[T],
293+
) -> Result<(), Box<dyn StdError>> {
291294
// Unquote and merge C string constants
292295
let unquote_re = regex::Regex::new(r#""(.*?[^\\])"\s*"#).unwrap();
293296
let unquote = |data: &str| -> String {
@@ -327,6 +330,11 @@ pub fn print_cargo_metadata<T: AsRef<Path>>(includes: &[T]) -> Result<(), Box<dy
327330
}
328331
}
329332

333+
println!(
334+
"cargo::metadata=build_dir={}",
335+
nginx.build_dir.to_str().expect("Unicode build path")
336+
);
337+
330338
println!(
331339
"cargo::metadata=include={}",
332340
// 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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ events {
1616

1717

1818
http {
19-
include mime.types;
19+
# include mime.types;
2020
default_type application/octet-stream;
2121

2222

0 commit comments

Comments
 (0)