From 0281acd96a9d5f7b3737c78a0cce499e830913c5 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 3 Dec 2024 17:17:03 -0500 Subject: [PATCH] Use `startup_file` to force a standardized `repos` option during testing (#650) * Tweak to use `not(unix)` to ensure we cover everything (Technically `wasm` is another valid option here) * Use `startup_file` to force a standardized `repos` option during testing --- crates/ark/src/fixtures/dummy_frontend.rs | 17 ++++++++--- crates/ark/src/repos.rs | 2 +- crates/ark/tests/repos-auto.rs | 14 ++++++++- crates/ark/tests/repos-conf-file.rs | 37 +++++++++++++++++++---- 4 files changed, 58 insertions(+), 12 deletions(-) diff --git a/crates/ark/src/fixtures/dummy_frontend.rs b/crates/ark/src/fixtures/dummy_frontend.rs index c1fc45ccf..5823dd16c 100644 --- a/crates/ark/src/fixtures/dummy_frontend.rs +++ b/crates/ark/src/fixtures/dummy_frontend.rs @@ -31,6 +31,7 @@ struct DummyArkFrontendOptions { r_environ: bool, session_mode: SessionMode, default_repos: DefaultRepos, + startup_file: Option, } /// Wrapper around `DummyArkFrontend` that uses `SessionMode::Notebook` @@ -110,7 +111,7 @@ impl DummyArkFrontend { connection_file, Some(registration_file), r_args, - None, + options.startup_file, options.session_mode, false, options.default_repos, @@ -182,10 +183,15 @@ impl DerefMut for DummyArkFrontendNotebook { impl DummyArkFrontendDefaultRepos { /// Lock a frontend with a default repos setting. /// + /// NOTE: `startup_file` is required because you typically want + /// to force `options(repos =)` to a fixed value for testing, regardless + /// of what the caller's default `repos` are set as (i.e. rig typically + /// sets it to a non-`@CRAN@` value). + /// /// NOTE: Only one `DummyArkFrontend` variant should call `lock()` within /// a given process. - pub fn lock(default_repos: DefaultRepos) -> Self { - Self::init(default_repos); + pub fn lock(default_repos: DefaultRepos, startup_file: String) -> Self { + Self::init(default_repos, startup_file); Self { inner: DummyArkFrontend::lock(), @@ -193,9 +199,11 @@ impl DummyArkFrontendDefaultRepos { } /// Initialize with given default repos - fn init(default_repos: DefaultRepos) { + fn init(default_repos: DefaultRepos, startup_file: String) { let mut options = DummyArkFrontendOptions::default(); options.default_repos = default_repos; + options.startup_file = Some(startup_file); + FRONTEND.get_or_init(|| Arc::new(Mutex::new(DummyArkFrontend::init(options)))); } } @@ -263,6 +271,7 @@ impl Default for DummyArkFrontendOptions { r_environ: false, session_mode: SessionMode::Console, default_repos: DefaultRepos::Auto, + startup_file: None, } } } diff --git a/crates/ark/src/repos.rs b/crates/ark/src/repos.rs index 508783653..b5ef6a094 100644 --- a/crates/ark/src/repos.rs +++ b/crates/ark/src/repos.rs @@ -111,7 +111,7 @@ fn apply_default_repos_auto() -> anyhow::Result<()> { } /// On Windows, we just use the RStudio CRAN mirror as the default. -#[cfg(windows)] +#[cfg(not(unix))] fn apply_default_repos_auto() -> anyhow::Result<()> { apply_default_repos(DefaultRepos::RStudio) } diff --git a/crates/ark/tests/repos-auto.rs b/crates/ark/tests/repos-auto.rs index 9d50a43a2..b9e29ca1d 100644 --- a/crates/ark/tests/repos-auto.rs +++ b/crates/ark/tests/repos-auto.rs @@ -5,6 +5,8 @@ // // +use std::io::Write; + use amalthea::fixtures::dummy_frontend::ExecuteRequestOptions; use ark::fixtures::DummyArkFrontendDefaultRepos; @@ -12,7 +14,17 @@ use ark::fixtures::DummyArkFrontendDefaultRepos; /// CRAN mirror. #[test] fn test_auto_repos() { - let frontend = DummyArkFrontendDefaultRepos::lock(ark::repos::DefaultRepos::Auto); + // Use a startup file to force a standardized `repos` on startup, + // regardless of what your local R version has set (i.e. from rig) + let contents = r#"options(repos = c(CRAN = "@CRAN@"))"#; + let mut startup_file = tempfile::NamedTempFile::new().unwrap(); + write!(startup_file, "{contents}").unwrap(); + let startup_path = startup_file.path(); + + let frontend = DummyArkFrontendDefaultRepos::lock( + ark::repos::DefaultRepos::Auto, + startup_path.to_str().unwrap().to_string(), + ); let code = r#"getOption("repos")[["CRAN"]]"#; frontend.send_execute_request(code, ExecuteRequestOptions::default()); diff --git a/crates/ark/tests/repos-conf-file.rs b/crates/ark/tests/repos-conf-file.rs index a8f8e302b..e31d68f74 100644 --- a/crates/ark/tests/repos-conf-file.rs +++ b/crates/ark/tests/repos-conf-file.rs @@ -9,7 +9,8 @@ use std::io::Write; use amalthea::fixtures::dummy_frontend::ExecuteRequestOptions; use ark::fixtures::DummyArkFrontendDefaultRepos; -/// Using a configuration file, set the default CRAN repo to a custom value. +/// Using a configuration file, set the default CRAN repo to a custom value, +/// and add an extra internal repo. #[test] fn test_conf_file_repos() { let contents = r#"# Custom CRAN repo configuration file @@ -17,12 +18,21 @@ fn test_conf_file_repos() { CRAN=https://my.cran.mirror/ Internal=https://internal.cran.mirror/ "#; - let mut file = tempfile::NamedTempFile::new().unwrap(); - write!(file, "{contents}").unwrap(); + let mut conf_file = tempfile::NamedTempFile::new().unwrap(); + write!(conf_file, "{contents}").unwrap(); + let conf_path = conf_file.path(); - let path = file.path(); - let frontend = - DummyArkFrontendDefaultRepos::lock(ark::repos::DefaultRepos::ConfFile(path.to_path_buf())); + // Use a startup file to force a standardized `repos` on startup, + // regardless of what your local R version has set (i.e. from rig) + let contents = r#"options(repos = c(CRAN = "@CRAN@"))"#; + let mut startup_file = tempfile::NamedTempFile::new().unwrap(); + write!(startup_file, "{contents}").unwrap(); + let startup_path = startup_file.path(); + + let frontend = DummyArkFrontendDefaultRepos::lock( + ark::repos::DefaultRepos::ConfFile(conf_path.to_path_buf()), + startup_path.to_str().unwrap().to_string(), + ); let code = r#"getOption("repos")[["CRAN"]]"#; frontend.send_execute_request(code, ExecuteRequestOptions::default()); @@ -37,5 +47,20 @@ Internal=https://internal.cran.mirror/ frontend.recv_iopub_idle(); + assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count); + + let code = r#"getOption("repos")[["Internal"]]"#; + frontend.send_execute_request(code, ExecuteRequestOptions::default()); + frontend.recv_iopub_busy(); + + let input = frontend.recv_iopub_execute_input(); + assert_eq!(input.code, code); + assert_eq!( + frontend.recv_iopub_execute_result(), + r#"[1] "https://internal.cran.mirror/""# + ); + + frontend.recv_iopub_idle(); + assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count) }