feat! drop support for limiting the duration of a CI run

This adapter is only ever run by the Radicle CI broker, and that can
now limit CI run duration itself. Thus having this adapter do it too
is pointless duplication.

Further, this removes the run time dependency on an external "timeout"
command, which turns out not to be portable: it isn't available on
Macs, by default.

Signed-off-by: Lars Wirzenius <liw@liw.fi>

README.md

@@ -55,7 +55,6 @@ The fields are:
 * `base_url` --- base URL for build logs (mandatory for access from CI broker page)
 * `state` --- required, directory where per-run directories are stored
 * `log` --- required, file where native CI should write a log
-* `timeout` --- optional, maximum duration of the run, in seconds
 
 
 ## Packaging

src/config.rs

@@ -4,8 +4,6 @@ use serde::{Deserialize, Serialize};
 
 use crate::logfile::{AdminLog, LogError};
 
-const DEFAULT_TIMEOUT: usize = 3600;
-
 /// Configuration file for `radicle-native-ci`.
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
@@ -17,9 +15,6 @@ pub struct Config {
     /// File where native CI should write a log.
     pub log: PathBuf,
 
-    /// Optional maximum duration of a CI run.
-    pub timeout: Option<usize>,
-
     /// Optional base URL to information about each run. The run ID is
     /// appended, with a slash if needed.
     #[serde(skip_serializing_if = "Option::is_none")]
@@ -43,17 +38,7 @@ impl Config {
     pub fn read(filename: &Path) -> Result<Self, ConfigError> {
         let file = std::fs::File::open(filename)
             .map_err(|e| ConfigError::ReadConfig(filename.into(), e))?;
-        let mut config: Self = serde_yaml::from_reader(&file)
-            .map_err(|e| ConfigError::ParseConfig(filename.into(), e))?;
-        if config.timeout.is_none() {
-            config.timeout = Some(DEFAULT_TIMEOUT);
-        }
-        Ok(config)
-    }
-
-    /// Return timeout or default.
-    pub fn timeout(&self) -> usize {
-        self.timeout.unwrap_or(DEFAULT_TIMEOUT)
+        serde_yaml::from_reader(&file).map_err(|e| ConfigError::ParseConfig(filename.into(), e))
     }
 
     /// Return configuration serialized to JSON.

src/engine.rs

@@ -219,7 +219,6 @@ impl Engine {
         run.set_request(req);
         run.set_commit(commit);
         run.set_storage(storage);
-        run.set_timeout(self.config.timeout());
 
         // Actually run. Examine the run log to decide if the run
         // succeeded or failed.

src/run.rs

@@ -15,9 +15,6 @@ use crate::{
 // Exit code to indicate we didn't get one from the process.
 const NO_EXIT: i32 = 999;
 
-// Exit code for the timeout command to indicate it timed out.
-const EXIT_TIMEOUT: i32 = 124;
-
 // Path to the repository's CI run specification. This is relative to
 // the root of the repository.
 pub const RUNSPEC_PATH: &str = ".radicle/native.yaml";
@@ -33,7 +30,6 @@ pub struct Run {
     repo_name: Option<String>,
     commit: Option<Oid>,
     storage: Option<PathBuf>,
-    timeout: Option<usize>,
     src: PathBuf,
 }
 
@@ -49,7 +45,6 @@ impl Run {
             request: None,
             commit: None,
             storage: None,
-            timeout: None,
             src: run_dir.join("src"),
         })
     }
@@ -75,13 +70,6 @@ impl Run {
         self.storage = Some(path.into());
     }
 
-    /// Optionally set the maximum duration to execute the shell
-    /// commands specified for a project, in seconds. If not set, a
-    /// generous built-in default is used.
-    pub fn set_timeout(&mut self, seconds: usize) {
-        self.timeout = Some(seconds);
-    }
-
     /// Run CI on a project.
     ///
     /// This runs CI for the project, and then clean up: persist the
@@ -131,7 +119,6 @@ impl Run {
         let request = self.request.clone().ok_or(RunError::Missing("request"))?;
         let commit = self.commit.ok_or(RunError::Missing("commit"))?;
         let storage = self.storage.as_ref().ok_or(RunError::Missing("storage"))?;
-        let timeout = self.timeout.ok_or(RunError::Missing("timeout"))?;
 
         // Record metadata in the run log.
         self.run_log.title("Log from Radicle native CI");
@@ -164,11 +151,7 @@ impl Run {
         };
 
         let snippet = format!("set -xeuo pipefail\n{}", &runspec.shell);
-        let timeout = format!("{}", timeout);
-        let exit = self.runcmd(&["timeout", &timeout, "bash", "-c", &snippet], &src)?;
-        if exit == EXIT_TIMEOUT {
-            self.run_log.command_timed_out();
-        }
+        self.runcmd(&["bash", "-c", &snippet], &src)?;
 
         Ok(())
     }

src/runlog.rs

@@ -24,7 +24,6 @@ pub struct RunLog {
     branch: Option<String>,
     patch: Option<(Oid, String)>,
     commands: Vec<Command>,
-    timeout: bool,
     runspec: Option<RunSpec>,
     runspec_error: Option<String>,
 }
@@ -63,7 +62,7 @@ impl RunLog {
     }
 
     pub fn all_commands_succeeded(&self) -> bool {
-        self.commands.iter().all(|c| c.exit == 0) && !self.timeout && self.runspec_error.is_none()
+        self.commands.iter().all(|c| c.exit == 0) && self.runspec_error.is_none()
     }
 
     pub fn runspec(&mut self, runspec: RunSpec) {
@@ -96,10 +95,6 @@ impl RunLog {
         });
     }
 
-    pub fn command_timed_out(&mut self) {
-        self.timeout = true;
-    }
-
     pub fn write(&self) -> Result<(), RunLogError> {
         let title = self.title.as_ref().ok_or(RunLogError::Missing("title"))?;
         let rid = self.rid.as_ref().ok_or(RunLogError::Missing("rid"))?;
@@ -167,11 +162,6 @@ impl RunLog {
             body.push_child(error);
         }
 
-        if self.timeout {
-            let timeout = Element::new(Tag::P).with_text("Last command timed out");
-            body.push_child(timeout);
-        }
-
         if let Some(runspec) = &self.runspec {
             let text = serde_yaml::to_string(&runspec).map_err(RunLogError::RunSpec)?;
             body.push_child(toc.push(Element::new(Tag::Span).with_child(

tests/integration.rs

@@ -36,10 +36,6 @@ use tempfile::{tempdir, TempDir};
 
 use radicle_native_ci::{config::Config, runspec::RunSpec};
 
-// Default timeout for CI runs. This is deliberately short to keep the
-// time to run tests short.
-const TIMEOUT: usize = 3;
-
 // The result of running the adapter.
 #[allow(dead_code)]
 struct AdapterResult {
@@ -367,7 +363,6 @@ impl TestCaseBuilder {
             let config = Config {
                 state: tmp.join("state"),
                 log: tmp.join("log.html"),
-                timeout: Some(TIMEOUT),
                 base_url: None,
             };
             let config = serde_yaml::to_string(&config).unwrap();
@@ -376,7 +371,6 @@ impl TestCaseBuilder {
             let config = Config {
                 state: tmp.join("state"),
                 log: tmp.join("log.html"),
-                timeout: Some(TIMEOUT),
                 base_url: Some(url.into()),
             };
             let config = serde_yaml::to_string(&config).unwrap();
@@ -704,22 +698,6 @@ fn commit_doesnt_exist() -> TestResult {
     Ok(())
 }
 
-// What does the adapter do if the commands its running take too long?
-#[test]
-fn command_takes_too_long() -> TestResult {
-    let result = TestCaseBuilder::new()?
-        .with_config()
-        .with_request_for_nonexistent_commit()
-        .with_shell(&format!("sleep {}", TIMEOUT + 2))
-        .build()?
-        .run()?;
-
-    result.assert_got_exit(1);
-    result.assert_got_run_id();
-    result.assert_got_failure();
-    Ok(())
-}
-
 // What does the adapter do if everything goes well?
 #[test]
 fn happy_path() -> TestResult {