Fix CommandLineTest port conflicts on CI

lighthouse/src/main.rs

@@ -688,9 +688,15 @@ fn run<E: EthSpec>(
             let executor = context.executor.clone();
             let mut config = beacon_node::get_config::<E>(matches, &context)?;
             config.logger_config = logger_config;
-            let shutdown_flag = matches.get_flag("immediate-shutdown");
             // Dump configs if `dump-config` or `dump-chain-config` flags are set
             clap_utils::check_dump_configs::<_, E>(matches, &config, &context.eth2_config.spec)?;
+
+            let shutdown_flag = matches.get_flag("immediate-shutdown");
+            if shutdown_flag {
+                info!(log, "Beacon node immediate shutdown triggered.");
+                return Ok(());
+            }
+
             executor.clone().spawn(
                 async move {
                     if let Err(e) = ProductionBeaconNode::new(context.clone(), config).await {
@@ -700,10 +706,6 @@ fn run<E: EthSpec>(
                         let _ = executor
                             .shutdown_sender()
                             .try_send(ShutdownReason::Failure("Failed to start beacon node"));
-                    } else if shutdown_flag {
-                        let _ = executor.shutdown_sender().try_send(ShutdownReason::Success(
-                            "Beacon node immediate shutdown triggered.",
-                        ));
                     }
                 },
                 "beacon_node",
@@ -715,31 +717,31 @@ fn run<E: EthSpec>(
             let executor = context.executor.clone();
             let config = validator_client::Config::from_cli(matches, context.log())
                 .map_err(|e| format!("Unable to initialize validator config: {}", e))?;
-            let shutdown_flag = matches.get_flag("immediate-shutdown");
             // Dump configs if `dump-config` or `dump-chain-config` flags are set
             clap_utils::check_dump_configs::<_, E>(matches, &config, &context.eth2_config.spec)?;
-            if !shutdown_flag {
-                executor.clone().spawn(
-                    async move {
-                        if let Err(e) = ProductionValidatorClient::new(context, config)
-                            .and_then(|mut vc| async move { vc.start_service().await })
-                            .await
-                        {
-                            crit!(log, "Failed to start validator client"; "reason" => e);
-                            // Ignore the error since it always occurs during normal operation when
-                            // shutting down.
-                            let _ = executor.shutdown_sender().try_send(ShutdownReason::Failure(
-                                "Failed to start validator client",
-                            ));
-                        }
-                    },
-                    "validator_client",
-                );
-            } else {
-                let _ = executor.shutdown_sender().try_send(ShutdownReason::Success(
-                    "Validator client immediate shutdown triggered.",
-                ));
+
+            let shutdown_flag = matches.get_flag("immediate-shutdown");
+            if shutdown_flag {
+                info!(log, "Validator client immediate shutdown triggered.");
+                return Ok(());
             }
+
+            executor.clone().spawn(
+                async move {
+                    if let Err(e) = ProductionValidatorClient::new(context, config)
+                        .and_then(|mut vc| async move { vc.start_service().await })
+                        .await
+                    {
+                        crit!(log, "Failed to start validator client"; "reason" => e);
+                        // Ignore the error since it always occurs during normal operation when
+                        // shutting down.
+                        let _ = executor
+                            .shutdown_sender()
+                            .try_send(ShutdownReason::Failure("Failed to start validator client"));
+                    }
+                },
+                "validator_client",
+            );
         }
         _ => {
             crit!(log, "No subcommand supplied. See --help .");

lighthouse/tests/beacon_node.rs

@@ -58,13 +58,22 @@ impl CommandLineTest {
 
     fn run_with_zero_port(&mut self) -> CompletedTest<Config> {
         // Required since Deneb was enabled on mainnet.
-        self.cmd.arg("--allow-insecure-genesis-sync");
-        self.run_with_zero_port_and_no_genesis_sync()
+        self.set_allow_insecure_genesis_sync()
+            .run_with_zero_port_and_no_genesis_sync()
     }
 
     fn run_with_zero_port_and_no_genesis_sync(&mut self) -> CompletedTest<Config> {
+        self.set_zero_port().run()
+    }
+
+    fn set_allow_insecure_genesis_sync(&mut self) -> &mut Self {
+        self.cmd.arg("--allow-insecure-genesis-sync");
+        self
+    }
+
+    fn set_zero_port(&mut self) -> &mut Self {
         self.cmd.arg("-z");
-        self.run()
+        self
     }
 }
 
@@ -101,9 +110,20 @@ fn staking_flag() {
 }
 
 #[test]
-#[should_panic]
 fn allow_insecure_genesis_sync_default() {
-    CommandLineTest::new().run_with_zero_port_and_no_genesis_sync();
+    CommandLineTest::new()
+        .run_with_zero_port_and_no_genesis_sync()
+        .with_config(|config| {
+            assert_eq!(config.allow_insecure_genesis_sync, false);
+        });
+}
+
+#[test]
+#[should_panic]
+fn insecure_genesis_sync_should_panic() {
+    CommandLineTest::new()
+        .set_zero_port()
+        .run_with_immediate_shutdown(false);
 }
 
 #[test]
@@ -2252,7 +2272,9 @@ fn ensure_panic_on_failed_launch() {
     CommandLineTest::new()
         .flag("slasher", None)
         .flag("slasher-chunk-size", Some("10"))
-        .run_with_zero_port()
+        .set_allow_insecure_genesis_sync()
+        .set_zero_port()
+        .run_with_immediate_shutdown(false)
         .with_config(|config| {
             let slasher_config = config
                 .slasher

lighthouse/tests/exec.rs

@@ -21,12 +21,19 @@ pub trait CommandLineTestExec {
         self
     }
 
+    fn run(&mut self) -> CompletedTest<Self::Config> {
+        self.run_with_immediate_shutdown(true)
+    }
+
     /// Executes the `Command` returned by `Self::cmd_mut` with temporary data directory, dumps
-    /// the configuration and shuts down immediately.
+    /// the configuration and shuts down immediately if `immediate_shutdown` is set to true.
     ///
     /// Options `--datadir`, `--dump-config`, `--dump-chain-config` and `--immediate-shutdown` must
     /// not be set on the command.
-    fn run(&mut self) -> CompletedTest<Self::Config> {
+    fn run_with_immediate_shutdown(
+        &mut self,
+        immediate_shutdown: bool,
+    ) -> CompletedTest<Self::Config> {
         // Setup temp directory.
         let tmp_dir = TempDir::new().expect("Unable to create temporary directory");
         let tmp_config_path: PathBuf = tmp_dir.path().join("config.json");
@@ -39,8 +46,11 @@ pub trait CommandLineTestExec {
             .arg(format!("--{}", "dump-config"))
             .arg(tmp_config_path.as_os_str())
             .arg(format!("--{}", "dump-chain-config"))
-            .arg(tmp_chain_config_path.as_os_str())
-            .arg("--immediate-shutdown");
+            .arg(tmp_chain_config_path.as_os_str());
+
+        if immediate_shutdown {
+            cmd.arg("--immediate-shutdown");
+        }
 
         // Run the command.
         let output = output_result(cmd);