Repository: static-web-server/static-web-server (language: rust)

Issue:
Tests fail because CLI options are parsed
### Search for duplicate issues

- [X] I already searched, and this issue is not a duplicate.

### Issue scope

CLI

### Describe the bug

Some tests load custom settings but also parse the CLI options. When tests are being run with parameters like `--test-threads 1`, some tests fail because static-web-server itself does not have such an option.

### How to reproduce it

`cargo test -- --test-threads 1`

### Expected behavior

Tests succeed.

### Complementary information

The option to specify the test threads is even mentioned in the Rust book: https://doc.rust-lang.org/book/ch11-02-running-tests.html#running-tests-in-parallel-or-consecutively

Failing log:
```
     Running tests/handler.rs (target/x86_64-unknown-linux-gnu/release/deps/handler-c38b80b7779dbedd)

running 2 tests
error: unexpected argument '--test-threads' found

  tip: a similar argument exists: '--security-headers'

Usage: handler-c38b80b7779dbedd <--host <HOST>|--port <PORT>|--fd <FD>|--threads-multiplier <THREADS_MULTIPLIER>|--max-blocking-threads <MAX_BLOCKING_THREADS>|--root <ROOT>|--page50x <PAGE50X>|--page404 <PAGE404>|--page-fallback <PAGE_FALLBACK>|--log-level <LOG_LEVEL>|--cors-allow-origins <CORS_ALLOW_ORIGINS>|--cors-allow-headers <CORS_ALLOW_HEADERS>|--cors-expose-headers <CORS_EXPOSE_HEADERS>|--http2[=<HTTP2>]|--http2-tls-cert <HTTP2_TLS_CERT>|--http2-tls-key <HTTP2_TLS_KEY>|--https-redirect[=<HTTPS_REDIRECT>]|--https-redirect-host <HTTPS_REDIRECT_HOST>|--https-redirect-from-port <HTTPS_REDIRECT_FROM_PORT>|--https-redirect-from-hosts <HTTPS_REDIRECT_FROM_HOSTS>|--index-files <INDEX_FILES>|--compression[=<COMPRESSION>]|--compression-level <COMPRESSION_LEVEL>|--compression-static[=<COMPRESSION_STATIC>]|--directory-listing[=<DIRECTORY_LISTING>]|--directory-listing-order <DIRECTORY_LISTING_ORDER>|--directory-listing-format <DIRECTORY_LISTING_FORMAT>|--security-headers[=<SECURITY_HEADERS>]|--cache-control-headers[=<CACHE_CONTROL_HEADERS>]|--basic-auth <BASIC_AUTH>|--grace-period <GRACE_PERIOD>|--config-file <CONFIG_FILE>|--log-remote-address[=<LOG_REMOTE_ADDRESS>]|--redirect-trailing-slash[=<REDIRECT_TRAILING_SLASH>]|--ignore-hidden-files[=<IGNORE_HIDDEN_FILES>]|--disable-symlinks[=<DISABLE_SYMLINKS>]|--health[=<HEALTH>]|--maintenance-mode[=<MAINTENANCE_MODE>]|--maintenance-mode-status <MAINTENANCE_MODE_STATUS>|--maintenance-mode-file <MAINTENANCE_MODE_FILE>|--version>

For more information, try '--help'.
error: unexpected argument '--test-threads' found

  tip: a similar argument exists: '--security-headers'

Usage: handler-c38b80b7779dbedd <--host <HOST>|--port <PORT>|--fd <FD>|--threads-multiplier <THREADS_MULTIPLIER>|--max-blocking-threads <MAX_BLOCKING_THREADS>|--root <ROOT>|--page50x <PAGE50X>|--page404 <PAGE404>|--page-fallback <PAGE_FALLBACK>|--log-level <LOG_LEVEL>|--cors-allow-origins <CORS_ALLOW_ORIGINS>|--cors-allow-headers <CORS_ALLOW_HEADERS>|--cors-expose-headers <CORS_EXPOSE_HEADERS>|--http2[=<HTTP2>]|--http2-tls-cert <HTTP2_TLS_CERT>|--http2-tls-key <HTTP2_TLS_KEY>|--https-redirect[=<HTTPS_REDIRECT>]|--https-redirect-host <HTTPS_REDIRECT_HOST>|--https-redirect-from-port <HTTPS_REDIRECT_FROM_PORT>|--https-redirect-from-hosts <HTTPS_REDIRECT_FROM_HOSTS>|--index-files <INDEX_FILES>|--compression[=<COMPRESSION>]|--compression-level <COMPRESSION_LEVEL>|--compression-static[=<COMPRESSION_STATIC>]|--directory-listing[=<DIRECTORY_LISTING>]|--directory-listing-order <DIRECTORY_LISTING_ORDER>|--directory-listing-format <DIRECTORY_LISTING_FORMAT>|--security-headers[=<SECURITY_HEADERS>]|--cache-control-headers[=<CACHE_CONTROL_HEADERS>]|--basic-auth <BASIC_AUTH>|--grace-period <GRACE_PERIOD>|--config-file <CONFIG_FILE>|--log-remote-address[=<LOG_REMOTE_ADDRESS>]|--redirect-trailing-slash[=<REDIRECT_TRAILING_SLASH>]|--ignore-hidden-files[=<IGNORE_HIDDEN_FILES>]|--disable-symlinks[=<DISABLE_SYMLINKS>]|--health[=<HEALTH>]|--maintenance-mode[=<MAINTENANCE_MODE>]|--maintenance-mode-status <MAINTENANCE_MODE_STATUS>|--maintenance-mode-file <MAINTENANCE_MODE_FILE>|--version>

For more information, try '--help'.
error: test failed, to rerun pass `--test handler`

Caused by:
  process didn't exit successfully: `/build/source/target/x86_64-unknown-linux-gnu/release/deps/handler-c38b80b7779dbedd --test-threads=24` (exit status: 2)
```

### Build target

Built from source (specify below)

### Environment and specs

- [x] **static-web-server:** v1.32.1

### Additional context

_No response_


Discussion / hints:
Unfortunately, our CLI parser does not seem to support fine-grained parsing skipping unknown arguments. https://github.com/clap-rs/clap/issues/1404
However, setting up a Rust environment variable like `RUST_TEST_THREADS=1` could be used as a workaround. Let me know if that works.
Thanks for the quick response.

Couldn't parsing arguments be disabled when the code is running in the tests? IMHO it doesn't make sense to parse the actual arguments of the test runner.

After some more investigation I was able to solve the issue with the following patch. Might something along those lines be an option?

```diff
diff --git a/src/bin/server.rs b/src/bin/server.rs
index 15e4e73..c3d6c0b 100644
--- a/src/bin/server.rs
+++ b/src/bin/server.rs
@@ -15,7 +15,7 @@ static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc;
 use static_web_server::{Result, Settings};
 
 fn main() -> Result {
-    let opts = Settings::get(true)?;
+    let opts = Settings::get(true, true)?;
 
     if opts.general.version {
         return static_web_server::settings::cli_output::display_version();
diff --git a/src/settings/mod.rs b/src/settings/mod.rs
index 5f9f76d..bb4e171 100644
--- a/src/settings/mod.rs
+++ b/src/settings/mod.rs
@@ -99,8 +99,8 @@ impl Settings {
     /// Reads CLI/Env and config file options returning the server settings.
     /// It also takes care to initialize the logging system with its level
     /// once the `general` settings are determined.
-    pub fn get(log_init: bool) -> Result<Settings> {
-        let opts = General::parse();
+    pub fn get(log_init: bool, parse_args: bool) -> Result<Settings> {
+        let opts = if parse_args { General::parse() } else { General::parse_from([""]) };
 
         // Define the general CLI/file options
         let version = opts.version;
diff --git a/src/testing.rs b/src/testing.rs
index 3c3459e..2d2c6f0 100644
--- a/src/testing.rs
+++ b/src/testing.rs
@@ -26,7 +26,7 @@ pub mod fixtures {
         // Replace default config file and load the fixture TOML settings
         let f = PathBuf::from("tests/fixtures").join(fixture_toml);
         std::env::set_var("SERVER_CONFIG_FILE", f);
-        Settings::get(false).unwrap()
+        Settings::get(false, false).unwrap()
     }
 
     /// Create a `RequestHandler` from a custom TOML config file (fixture).
diff --git a/src/winservice.rs b/src/winservice.rs
index 64dab9d..275ba6d 100644
--- a/src/winservice.rs
+++ b/src/winservice.rs
@@ -73,7 +73,7 @@ fn set_service_state(
 
 fn run_service() -> Result {
     // Log is already initialized so there is no need to do it again.
-    let opts = Settings::get(false)?;
+    let opts = Settings::get(false, true)?;
 
     tracing::info!("windows service: starting service setup");
 
```

> Couldn't parsing arguments be disabled when the code is running in the tests? IMHO it doesn't make sense to parse the actual arguments of the test runner.

Sorry, now I see what you mean. I was a bit blinded.
Yes, it can be an option. In fact, this could be simplified like this:

```diff
diff --git a/src/settings/mod.rs b/src/settings/mod.rs
index 5f9f76d..c2b4141 100644
--- a/src/settings/mod.rs
+++ b/src/settings/mod.rs
@@ -100,7 +100,20 @@ impl Settings {
     /// It also takes care to initialize the logging system with its level
     /// once the `general` settings are determined.
     pub fn get(log_init: bool) -> Result<Settings> {
-        let opts = General::parse();
+        Self::read(log_init, true)
+    }
+
+    /// Reads CLI/Env and config file options returning the server settings without parsing arguments useful for testing.
+    pub fn get_unparsed(log_init: bool) -> Result<Settings> {
+        Self::read(log_init, false)
+    }
+
+    fn read(log_init: bool, parse_args: bool) -> Result<Settings> {
+        let opts = if parse_args {
+            General::parse()
+        } else {
+            General::parse_from([""])
+        };
 
         // Define the general CLI/file options
         let version = opts.version;
diff --git a/src/testing.rs b/src/testing.rs
index 3c3459e..b5e4276 100644
--- a/src/testing.rs
+++ b/src/testing.rs
@@ -26,7 +26,7 @@ pub mod fixtures {
         // Replace default config file and load the fixture TOML settings
         let f = PathBuf::from("tests/fixtures").join(fixture_toml);
         std::env::set_var("SERVER_CONFIG_FILE", f);
-        Settings::get(false).unwrap()
+        Settings::get_unparsed(false).unwrap()
     }
 
     /// Create a `RequestHandler` from a custom TOML config file (fixture).
```

Feel free to submit a PR.

Fix patch:
diff --git a/src/settings/mod.rs b/src/settings/mod.rs
index 5f9f76d7..7666b295 100644
--- a/src/settings/mod.rs
+++ b/src/settings/mod.rs
@@ -100,7 +100,21 @@ impl Settings {
     /// It also takes care to initialize the logging system with its level
     /// once the `general` settings are determined.
     pub fn get(log_init: bool) -> Result<Settings> {
-        let opts = General::parse();
+        Self::read(log_init, true)
+    }
+
+    /// Reads CLI/Env and config file options returning the server settings
+    /// without parsing arguments useful for testing.
+    pub fn get_unparsed(log_init: bool) -> Result<Settings> {
+        Self::read(log_init, false)
+    }
+
+    fn read(log_init: bool, parse_args: bool) -> Result<Settings> {
+        let opts = if parse_args {
+            General::parse()
+        } else {
+            General::parse_from([""])
+        };
 
         // Define the general CLI/file options
         let version = opts.version;


Test patch:
diff --git a/src/testing.rs b/src/testing.rs
index 3c3459e0..b5e4276f 100644
--- a/src/testing.rs
+++ b/src/testing.rs
@@ -26,7 +26,7 @@ pub mod fixtures {
         // Replace default config file and load the fixture TOML settings
         let f = PathBuf::from("tests/fixtures").join(fixture_toml);
         std::env::set_var("SERVER_CONFIG_FILE", f);
-        Settings::get(false).unwrap()
+        Settings::get_unparsed(false).unwrap()
     }
 
     /// Create a `RequestHandler` from a custom TOML config file (fixture).
diff --git a/tests/compression.rs b/tests/compression.rs
index 79d593cd..ebd80d94 100644
--- a/tests/compression.rs
+++ b/tests/compression.rs
@@ -3,6 +3,7 @@
 #![deny(rust_2018_idioms)]
 #![deny(dead_code)]
 
+#[cfg(test)]
 pub mod tests {
     use headers::HeaderValue;
     use hyper::Request;
diff --git a/tests/handler.rs b/tests/handler.rs
index 669b4431..d53c6585 100644
--- a/tests/handler.rs
+++ b/tests/handler.rs
@@ -3,6 +3,7 @@
 #![deny(rust_2018_idioms)]
 #![deny(dead_code)]
 
+#[cfg(test)]
 pub mod tests {
     use headers::HeaderValue;
     use hyper::Request;
diff --git a/tests/redirects.rs b/tests/redirects.rs
index c499ab9f..50ffce04 100644
--- a/tests/redirects.rs
+++ b/tests/redirects.rs
@@ -3,6 +3,7 @@
 #![deny(rust_2018_idioms)]
 #![deny(dead_code)]
 
+#[cfg(test)]
 pub mod tests {
     use hyper::Request;
     use std::net::SocketAddr;
diff --git a/tests/rewrites.rs b/tests/rewrites.rs
index d33b5a3b..3ede500e 100644
--- a/tests/rewrites.rs
+++ b/tests/rewrites.rs
@@ -3,6 +3,7 @@
 #![deny(rust_2018_idioms)]
 #![deny(dead_code)]
 
+#[cfg(test)]
 pub mod tests {
     use hyper::Request;
     use std::net::SocketAddr;