Skip to content

Commit 439bc5a

Browse files
Stabilize OMP version gate fixtures
agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@e4789b0
1 parent a601331 commit 439bc5a

1 file changed

Lines changed: 79 additions & 24 deletions

File tree

src/omp_session.rs

Lines changed: 79 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -260,47 +260,102 @@ fn channel_env(
260260
#[cfg(test)]
261261
mod tests {
262262
use super::*;
263-
use std::os::unix::fs::PermissionsExt as _;
263+
use std::collections::HashSet;
264+
use std::path::{Path, PathBuf};
265+
use std::sync::Barrier;
266+
267+
struct FakeExecutable {
268+
_directory: tempfile::TempDir,
269+
path: PathBuf,
270+
}
271+
272+
impl FakeExecutable {
273+
fn new(body: &str) -> Self {
274+
let directory = tempfile::Builder::new()
275+
.prefix("st2-omp-version-")
276+
.tempdir()
277+
.unwrap();
278+
let source = directory.path().join("omp.source");
279+
let path = directory.path().join("omp");
280+
std::fs::write(&source, body).unwrap();
281+
// A writer opened by one libtest thread is inherited by a child forked concurrently
282+
// from another, which can make the writer's later exec fail with ETXTBSY even after
283+
// the parent closes it. Let `install` create and close the executable in its own child:
284+
// the test process never owns a writable descriptor for the file it will execute.
285+
let output = std::process::Command::new("install")
286+
.args(["-m", "755"])
287+
.arg(&source)
288+
.arg(&path)
289+
.output()
290+
.unwrap();
291+
assert!(output.status.success(), "install failed: {output:?}");
292+
Self {
293+
_directory: directory,
294+
path,
295+
}
296+
}
297+
298+
fn path(&self) -> &Path {
299+
&self.path
300+
}
301+
}
264302

265303
#[test]
266304
fn version_gate_admits_the_verified_major() {
267-
let dir = tempfile::tempdir().unwrap();
268-
let fake = dir.path().join("omp");
269-
std::fs::write(&fake, "#!/bin/sh\nprintf 'omp v18.0.3\\n18.0.3\\n'\n").unwrap();
270-
// Keep the file writable so dropping the shebang bit back is unnecessary; make it
271-
// executable in place.
272-
std::fs::set_permissions(&fake, std::fs::Permissions::from_mode(0o755)).unwrap();
273-
assert!(verify_supported_version(fake.to_str().unwrap()).is_ok());
305+
let fake =
306+
FakeExecutable::new("#!/bin/sh\nprintf 'omp v18.0.3\\n18.0.3\\n'\n");
307+
verify_supported_version(fake.path().to_str().unwrap()).unwrap();
274308
}
275309

276310
#[test]
277311
fn version_gate_refuses_an_unverified_minor() {
278-
let dir = tempfile::tempdir().unwrap();
279-
let fake = dir.path().join("omp");
280-
std::fs::write(&fake, "#!/bin/sh\nprintf '18.1.0\\n'\n").unwrap();
281-
use std::os::unix::fs::PermissionsExt as _;
282-
std::fs::set_permissions(&fake, std::fs::Permissions::from_mode(0o755)).unwrap();
283-
let error = verify_supported_version(fake.to_str().unwrap()).unwrap_err();
312+
let fake = FakeExecutable::new("#!/bin/sh\nprintf '18.1.0\\n'\n");
313+
let error = verify_supported_version(fake.path().to_str().unwrap()).unwrap_err();
284314
assert!(error.to_string().contains("unverified"), "{error}");
285315
}
286316

287317
#[test]
288318
fn version_gate_refuses_an_unverified_major() {
289-
let dir = tempfile::tempdir().unwrap();
290-
let fake = dir.path().join("omp");
291-
std::fs::write(&fake, "#!/bin/sh\nprintf '19.0.1\\n'\n").unwrap();
292-
std::fs::set_permissions(&fake, std::fs::Permissions::from_mode(0o755)).unwrap();
293-
let error = verify_supported_version(fake.to_str().unwrap()).unwrap_err();
319+
let fake = FakeExecutable::new("#!/bin/sh\nprintf '19.0.1\\n'\n");
320+
let error = verify_supported_version(fake.path().to_str().unwrap()).unwrap_err();
294321
assert!(error.to_string().contains("unverified"), "{error}");
295322
}
296323

297324
#[test]
298325
fn version_gate_refuses_garbled_output() {
299-
let dir = tempfile::tempdir().unwrap();
300-
let fake = dir.path().join("omp");
301-
std::fs::write(&fake, "#!/bin/sh\nprintf 'not-a-version\\n'\n").unwrap();
302-
std::fs::set_permissions(&fake, std::fs::Permissions::from_mode(0o755)).unwrap();
303-
assert!(verify_supported_version(fake.to_str().unwrap()).is_err());
326+
let fake = FakeExecutable::new("#!/bin/sh\nprintf 'not-a-version\\n'\n");
327+
assert!(verify_supported_version(fake.path().to_str().unwrap()).is_err());
328+
}
329+
330+
#[test]
331+
fn version_gate_fixtures_are_parallel_safe() {
332+
const WORKERS: usize = 8;
333+
const ROUNDS: usize = 16;
334+
335+
let barrier = Barrier::new(WORKERS);
336+
let paths = std::thread::scope(|scope| {
337+
let mut workers = Vec::with_capacity(WORKERS);
338+
for _ in 0..WORKERS {
339+
let barrier = &barrier;
340+
workers.push(scope.spawn(move || {
341+
let mut paths = Vec::with_capacity(ROUNDS);
342+
barrier.wait();
343+
for _ in 0..ROUNDS {
344+
let fake = FakeExecutable::new("#!/bin/sh\nprintf '18.1.0\\n'\n");
345+
paths.push(fake.path().to_path_buf());
346+
let error =
347+
verify_supported_version(fake.path().to_str().unwrap()).unwrap_err();
348+
assert!(error.to_string().contains("unverified"), "{error}");
349+
}
350+
paths
351+
}));
352+
}
353+
workers
354+
.into_iter()
355+
.flat_map(|worker| worker.join().unwrap())
356+
.collect::<Vec<_>>()
357+
});
358+
assert_eq!(paths.iter().collect::<HashSet<_>>().len(), paths.len());
304359
}
305360

306361
#[test]

0 commit comments

Comments
 (0)