mononoke: fix getdeps/cargo double log init crash

Summary:
Cargo builds run multiple tests in same process, which was causing a failure since MononokeMatches started doing the one time log::set_boxed_logger() call.

I made MononokeMatches:new() error rather than panic in this case as it was already returning a Result anyway.

The failing test was only testing a very small section of code in block_execute, so this change removes it (its covered by integration test).  It also gave some MononokeMatches coverage as a side effect (also covered by integration test).

Reviewed By: krallin

Differential Revision: D27891187

fbshipit-source-id: 9787029b610040cbf0125ab79748d6a3e540d2ae

eden/mononoke/cmdlib/src/args/matches.rs

@@ -76,6 +76,7 @@ pub struct MononokeMatches<'a> {
 }
 
 impl<'a> MononokeMatches<'a> {
+    /// Due to global log init this can be called only once per process and will error otherwise
     pub fn new(
         fb: FacebookInit,
         matches: ArgMatches<'a>,
@@ -312,7 +313,7 @@ fn create_root_log_drain(fb: FacebookInit, matches: &ArgMatches<'_>) -> Result<i
     // NOTE: We pass an unfiltered Logger to init_stdlog_once. That's because we do the filtering
     // at the stdlog level there.
     let stdlog_logger = Logger::root(root_log_drain.clone(), o![]);
-    let stdlog_level = log::init_stdlog_once(stdlog_logger.clone(), stdlog_env);
+    let stdlog_level = log::init_stdlog_once(stdlog_logger.clone(), stdlog_env)?;
     trace!(
         stdlog_logger,
         "enabled stdlog with level: {:?} (set {} to configure)",

eden/mononoke/cmdlib/src/helpers.rs

@@ -321,50 +321,3 @@ where
         format_err!("Execution failed")
     })
 }
-
-#[cfg(test)]
-mod test {
-    use super::*;
-    use crate::args;
-    use anyhow::Error;
-    use futures::future::lazy;
-    use slog_glog_fmt;
-
-    use crate::monitoring::AliveService;
-
-    fn create_logger() -> Logger {
-        slog_glog_fmt::facebook_logger().unwrap()
-    }
-
-    fn exec_matches<'a>(fb: FacebookInit) -> MononokeMatches<'a> {
-        let app = args::MononokeAppBuilder::new("test_app").build();
-        let arg_vec = vec![
-            "test_prog",
-            "--skip-caching",
-            "--mononoke-config-path",
-            "/tmp/testpath",
-            "--local-configerator-path",
-            "/tmp/testpath",
-            "--disable-tunables",
-        ];
-        app.get_matches_from(fb, arg_vec).unwrap()
-    }
-
-    #[fbinit::test]
-    fn test_block_execute_success(fb: FacebookInit) {
-        let future = lazy(|_| -> Result<(), Error> { Ok(()) });
-        let logger = create_logger();
-        let matches = exec_matches(fb);
-        let res = block_execute(future, fb, "test_app", &logger, &matches, AliveService);
-        assert!(res.is_ok());
-    }
-
-    #[fbinit::test]
-    fn test_block_execute_error(fb: FacebookInit) {
-        let future = lazy(|_| -> Result<(), Error> { Err(Error::msg("Some error")) });
-        let logger = create_logger();
-        let matches = exec_matches(fb);
-        let res = block_execute(future, fb, "test_app", &logger, &matches, AliveService);
-        assert!(res.is_err());
-    }
-}

eden/mononoke/cmdlib/src/log.rs

@@ -5,6 +5,7 @@
  * GNU General Public License version 2.
  */
 
+use anyhow::Error;
 use env_logger::filter::{Builder, Filter};
 use log;
 use slog::{BorrowedKV, Level, Logger, SingleKV};
@@ -78,16 +79,16 @@ impl log::Log for LinkedLogger {
 }
 
 /// Wire up a slog Logger as the destination for std logs as per an env_logger filter spec. This
-/// sets the global logger, so it'll panic if called more than once.
-pub fn init_stdlog_once(logger: Logger, var: &str) -> log::LevelFilter {
+/// sets the global logger, so it'll error if called more than once.
+pub fn init_stdlog_once(logger: Logger, var: &str) -> Result<log::LevelFilter, Error> {
     // NOTE: The default level is ERROR, which should be fairly reasonable.
     let filter = Builder::from_env(var).build();
     let level = filter.filter();
 
-    log::set_boxed_logger(Box::new(LinkedLogger { logger, filter })).unwrap();
+    log::set_boxed_logger(Box::new(LinkedLogger { logger, filter }))?;
 
     // set_max_level ensures we don't produce logs that won't pass any filter at all.
     log::set_max_level(level);
 
-    level
+    Ok(level)
 }