From 77ae92267512b6a2b4b962f390be6e866beb88df Mon Sep 17 00:00:00 2001 From: Nicolas BACQUEY Date: Thu, 24 Oct 2024 16:41:13 +0200 Subject: [PATCH] Load JSON and TOML grammars before tests in `cli-testers.rs` We need to prefetch JSON and TOML grammars before running the tests, on pain of race condition: If multiple calls to Topiary are made in parallel and the grammar is missing, they will all try to fetch and build it, thus creating an empty .so file while g++ is running. If another instance of Topiary starts at this moment, it will mistake the empty .so file for an already built grammar, and try to run with it, resulting in an error. --- CHANGELOG.md | 3 +++ topiary-cli/tests/cli-tester.rs | 40 ++++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39c83a9e..dc89dd11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,9 @@ This name should be decided amongst the team before the release. [Full list of changes](https://github.com/tweag/topiary/compare/v0.5.1...HEAD) +### Fixed +- [#779](https://github.com/tweag/topiary/pull/779) Load relevant grammars before CLI tests + ## v0.5.1 - Fragrant Frangipani - 2024-10-22 [Full list of changes](https://github.com/tweag/topiary/compare/v0.4.0...v0.5.1) diff --git a/topiary-cli/tests/cli-tester.rs b/topiary-cli/tests/cli-tester.rs index 61089810..6d7bdafe 100644 --- a/topiary-cli/tests/cli-tester.rs +++ b/topiary-cli/tests/cli-tester.rs @@ -1,4 +1,4 @@ -use std::{fmt, fs, fs::File, io::Write, path::PathBuf}; +use std::{fmt, fs, fs::File, io::Write, path::PathBuf, sync::Once}; use assert_cmd::Command; use predicates::{ @@ -17,6 +17,35 @@ const TOML_INPUT: &str = r#" test= 123"#; const TOML_EXPECTED: &str = r#"test = 123 "#; +// We need to prefetch JSON and TOML grammars before running the tests, on pain of race condition: +// If multiple calls to Topiary are made in parallel and the grammar is missing, they will all try +// to fetch and build it, thus creating an empty .so file while g++ is running. If another instance +// of topiary starts at this moment, it will mistake the empty .so file for an already built grammar, +// and try to run with it, resulting in an error. See https://github.com/tweag/topiary/issues/767 +static INIT: Once = Once::new(); +pub fn initialize() { + INIT.call_once(|| { + #[cfg(feature = "json")] + Command::cargo_bin("topiary") + .expect("Unable to build Topiary") + .arg("fmt") + .arg("--language") + .arg("json") + .write_stdin("") + .assert() + .success(); + #[cfg(feature = "toml")] + Command::cargo_bin("topiary") + .expect("Unable to build Topiary") + .arg("fmt") + .arg("--language") + .arg("toml") + .write_stdin("") + .assert() + .success(); + }); +} + // The TempDir member of the State is not actually used. // However, removing it means that the directory is dropped at the end of the new() function, which causes it to be deleted. // This causes the path to the state file to be invalid and breaks the tests. @@ -47,6 +76,7 @@ impl State { #[test] #[cfg(feature = "json")] fn test_fmt_stdin() { + initialize(); let mut topiary = Command::cargo_bin("topiary").unwrap(); topiary @@ -63,6 +93,7 @@ fn test_fmt_stdin() { #[test] #[cfg(feature = "json")] fn test_fmt_stdin_query() { + initialize(); let mut topiary = Command::cargo_bin("topiary").unwrap(); topiary @@ -81,6 +112,7 @@ fn test_fmt_stdin_query() { #[test] #[cfg(feature = "json")] fn test_fmt_stdin_query_fallback() { + initialize(); let mut topiary = Command::cargo_bin("topiary").unwrap(); topiary @@ -99,6 +131,7 @@ fn test_fmt_stdin_query_fallback() { #[test] #[cfg(all(feature = "json", feature = "toml"))] fn test_fmt_files() { + initialize(); let json = State::new(JSON_INPUT, "json"); let toml = State::new(TOML_INPUT, "toml"); @@ -119,6 +152,7 @@ fn test_fmt_files() { #[test] #[cfg(all(feature = "json", feature = "toml"))] fn test_fmt_files_query_fallback() { + initialize(); let json = State::new(JSON_INPUT, "json"); let toml = State::new(TOML_INPUT, "toml"); @@ -141,6 +175,7 @@ fn test_fmt_files_query_fallback() { #[test] #[cfg(feature = "json")] fn test_fmt_dir() { + initialize(); let json = State::new(JSON_INPUT, "json"); let mut topiary = Command::cargo_bin("topiary").unwrap(); @@ -158,6 +193,7 @@ fn test_fmt_dir() { #[test] #[cfg(feature = "json")] fn test_fmt_invalid() { + initialize(); let mut topiary = Command::cargo_bin("topiary").unwrap(); // Can't specify --language with input files @@ -183,6 +219,7 @@ fn test_fmt_invalid() { #[test] #[cfg(feature = "json")] fn test_vis() { + initialize(); let mut topiary = Command::cargo_bin("topiary").unwrap(); // Sanity check output is a valid DOT graph @@ -202,6 +239,7 @@ fn test_vis() { #[test] #[cfg(feature = "json")] fn test_vis_invalid() { + initialize(); let mut topiary = Command::cargo_bin("topiary").unwrap(); // Can't specify --language with input file