Avoid relying on NeedsReport status for experiments

Updates the places which match on this status to instead use the condition which
produces needs-report (i.e., completed == expected total).

Author: Mark-Simulacrum

src/cli.rs

@@ -506,8 +506,6 @@ impl Crater {
                     );
                     workspace.purge_all_build_dirs()?;
                     res?;
-
-                    experiment.set_status(&db, Status::NeedsReport)?;
                 } else {
                     bail!("missing experiment {}", ex.0);
                 }
@@ -522,18 +520,18 @@ impl Crater {
                 let db = Database::open()?;
 
                 if let Some(mut experiment) = Experiment::get(&db, &ex.0)? {
-                    // Update the status
-                    match (experiment.status, force) {
-                        (Status::NeedsReport, _) | (Status::ReportFailed, _) | (_, true) => {
-                            experiment.set_status(&db, Status::GeneratingReport)?;
-                        }
-                        (other, false) => bail!(
-                            "can't generate the report of an experiment with status {} \
+                    let (completed, all) = experiment.raw_progress(&db)?;
+                    if !force && completed != all {
+                        bail!(
+                            "can't generate the report of an incomplete experiment: {}/{} results \
                              (use --force to override)",
-                            other
-                        ),
+                            completed,
+                            all,
+                        );
                     }
 
+                    experiment.set_status(&db, Status::GeneratingReport)?;
+
                     let result_db = DatabaseDB::new(&db);
                     let res = report::gen(
                         &result_db,
@@ -564,18 +562,18 @@ impl Crater {
                 let db = Database::open()?;
 
                 if let Some(mut experiment) = Experiment::get(&db, &ex.0)? {
-                    // Update the status
-                    match (experiment.status, force) {
-                        (Status::NeedsReport, _) | (Status::ReportFailed, _) | (_, true) => {
-                            experiment.set_status(&db, Status::GeneratingReport)?;
-                        }
-                        (other, false) => bail!(
-                            "can't publish the report of an experiment with status {} \
+                    let (completed, all) = experiment.raw_progress(&db)?;
+                    if !force && completed != all {
+                        bail!(
+                            "can't publish the report of an incomplete experiment: {}/{} results \
                              (use --force to override)",
-                            other
-                        ),
+                            completed,
+                            all,
+                        );
                     }
 
+                    experiment.set_status(&db, Status::GeneratingReport)?;
+
                     let result_db = DatabaseDB::new(&db);
                     let client = report::get_client_for_bucket(&s3_prefix.bucket)?;
 

src/experiments.rs

@@ -305,6 +305,17 @@ impl Experiment {
     pub fn ready_for_report(db: &Database) -> Fallible<Option<Experiment>> {
         let unfinished = Self::unfinished(db)?;
         for ex in unfinished {
+            if ex.status == Status::ReportFailed {
+                // Skip experiments whose report failed to generate. This avoids
+                // constantly retrying reports (and posting a message each time
+                // about the attempt); the retry-report command can override the
+                // failure state. In practice we rarely *fail* to generate
+                // reports in a clean way (instead OOMing or panicking, in which
+                // case it is fine to automatically retry the report, as we've
+                // not posted anything on GitHub -- it may be a problem from a
+                // performance perspective but no more than that).
+                continue;
+            }
             let (completed, all) = ex.raw_progress(db)?;
             if completed == all {
                 return Ok(Some(ex));