test: introduce env variable mocking (starship#1490)

Author: Tilmann Meyer

.github/workflows/workflow.yml

@@ -125,12 +125,6 @@ jobs:
           profile: minimal
           override: true
 
-      # Install dotnet at a fixed version
-      - name: Setup | DotNet
-        uses: actions/setup-dotnet@v1
-        with:
-          dotnet-version: "2.2.402"
-
       # Install Mercurial (pre-installed on Linux and windows)
       - name: Setup | Mercurial (macos)
         if: matrix.os == 'macOS-latest'

CONTRIBUTING.md

@@ -24,6 +24,52 @@ The project begins in [`main.rs`](src/main.rs), where the appropriate `print::`
 
 Any styling that is applied to a module is inherited by its segments. Module prefixes and suffixes by default don't have any styling applied to them.
 
+## Environment Variables and external commands
+
+We have custom functions to be able to test our modules better. Here we show you how.
+
+### Environment Variables
+
+To get an environment variable we have special function to allow for mocking of vars. Here's a quick example:
+
+```rust
+use super::{Context, Module, RootModuleConfig};
+
+use crate::configs::php::PhpConfig;
+use crate::formatter::StringFormatter;
+use crate::utils;
+
+
+pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
+   // Here `my_env_var` will be either the contents of the var or the function
+   // will exit if the variable is not set.
+   let my_env_var = context.get_env("MY_VAR")?;
+
+   // Then you can happily use the value
+}
+```
+
+## External commands
+
+To run a external command (e.g. to get the version of a tool) and to allow for mocking use the `utils::exec_cmd` function. Here's a quick example:
+
+```rust
+use super::{Context, Module, RootModuleConfig};
+
+use crate::configs::php::PhpConfig;
+use crate::formatter::StringFormatter;
+use crate::utils;
+
+
+pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
+   // Here `my_env_var` will be either the stdout of the called command or the function
+   // will exit if the called program was not installed or could not be run.
+   let output = utils::exec_cmd("my_command", &["first_arg", "second_arg"])?.stdout;
+
+   // Then you can happily use the output
+}
+```
+
 ## Logging
 
 Debug logging in starship is done with [pretty_env_logger](https://crates.io/crates/pretty_env_logger).
@@ -56,37 +102,81 @@ rustup component add rustfmt
 cargo fmt
 ```
 
-
 ## Testing
 
 Testing is critical to making sure starship works as intended on systems big and small. Starship interfaces with many applications and system APIs when generating the prompt, so there's a lot of room for bugs to slip in.
 
-Unit tests and a subset of integration tests can be run with `cargo test`.
-The full integration test suite is run on GitHub as part of our GitHub Actions continuous integration.
+Unit tests are written using the built-in Rust testing library in the same file as the implementation, as is traditionally done in Rust codebases. These tests can be run with `cargo test` and are run on GitHub as part of our GitHub Actions continuous integration to ensure consistend behavior.
+
+All tests that test the rendered output of a module should use `ModuleRenderer`. For Example:
+
+```rust
+use super::{Context, Module, RootModuleConfig};
+
+use crate::configs::php::PhpConfig;
+use crate::formatter::StringFormatter;
+use crate::utils;
+
+
+pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
+   /* This is where your module code goes */
+}
+
+#[cfg(test)]
+mod tests {
+   use super::*;
+   use crate::test::ModuleRenderer;
+   use ansi_term::Color;
+   use std::fs::File;
+   use std::io;
+
+
+   #[test]
+   fn should_render() -> io::Result<()> {
+      // Here you setup the testing environment
+      let tempdir = tempfile::tempdir()?;
+      // Create some file needed to render the module
+      File::create(dir.path().join("YOUR_FILE"))?.sync_all()?;
+
+      // The output of the module
+      let actual = ModuleRenderer::new("YOUR_MODULE_NAME")
+         // For a custom path
+         .path(&tempdir.path())
+         // For a custom config
+         .config(toml::toml!{
+            [YOUR_MODULE_NAME]
+            val = 1
+         })
+         // For env mocking
+         .env("KEY","VALUE")
+         // Run the module and collect the output
+         .collect();
+
+      // The value that should be rendered by the module.
+      let expected = Some(format!("{} ",Color::Black.paint("THIS SHOULD BE RENDERED")));
+
+      // Assert that the actual and expected values are the same
+      assert_eq!(actual, expected);
+
+      // Close the tempdir
+      tempdir.close()
+   }
+}
+```
 
-### Unit Testing
+If a module depends on output of another program, then that output should be added to the match statement in [`utils.rs`](src/utils.rs). The match has to be exactly the same as the call to `utils::exec_cmd()`, including positional arguments and flags. The array of arguments are joined by a `" "`, so `utils::exec_cmd("program", &["arg", "more_args"])` would match with the `program arg more_args` match statement.
 
-Unit tests are written using the built-in Rust testing library in the same file as the implementation, as is traditionally done in Rust codebases. These tests can be run with `cargo test`.
+If the program cannot be mocked (e.g. It performs some filesystem operations, either writing or reading files) then it has to added to the project's GitHub Actions workflow file([`.github/workflows/workflow.yml`](.github/workflows/workflow.yml)) and the test has to be marked with an `#[ignored]`. This ensures that anyone can run the test suite locally without needing to pre-configure their environment. The `#[ignored]` attribute is bypassed during CI runs in GitHub Actions.
 
 Unit tests should be fully isolated, only testing a given function's expected output given a specific input, and should be reproducible on any machine. Unit tests should not expect the computer running them to be in any particular state. This includes having any applications pre-installed, having any environment variables set, etc.
 
 The previous point should be emphasized: even seemingly innocuous ideas like "if we can see the directory, we can read it" or "nobody will have their home directory be a git repo" have bitten us in the past. Having even a single test fail can completely break installation on some platforms, so be careful with tests!
 
-### Integration Testing
-
-Integration tests are located in the [`tests/`](tests) directory and are also written using the built-in Rust testing library.
-
-Integration tests should test full modules or the entire prompt. All integration tests that expect the testing environment to have pre-existing state or tests that make permanent changes to the filesystem should have the `#[ignore]` attribute added to them. All tests that don't depend on any preexisting state will be run alongside the unit tests with `cargo test`.
-
-For tests that depend on having preexisting state, whatever needed state will have to be added to the project's GitHub Actions workflow file([`.github/workflows/workflow.yml`](.github/workflows/workflow.yml)).
-
 ### Test Programming Guidelines
 
 Any tests that depend on File I/O should use [`sync_all()`](https://doc.rust-lang.org/std/fs/struct.File.html#method.sync_all) when creating files or after writing to files.
 
-Any tests that use `tempfile::tempdir` should take care to call `dir.close()` after usage to ensure the lifecycle of the directory can be reasoned about.
-
-Any tests that use `create_fixture_repo()` should remove the returned directory after usage with  `remove_dir_all::remove_dir_all()`.
+Any tests that use `tempfile::tempdir` should take care to call `dir.close()` after usage to ensure the lifecycle of the directory can be reasoned about. This includes `fixture_repo()` as it returns a TempDir that should be closed.
 
 ## Running the Documentation Website Locally
 
@@ -98,17 +188,20 @@ After cloning the project, you can do the following to run the VuePress website
 
 1. `cd` into the `/docs` directory.
 2. Install the project dependencies:
-```
-$ npm install
-```
+
+   ```sh
+   npm install
+   ```
+
 3. Start the project in development mode:
-```
-$ npm run dev
-```
 
-Once setup is complete, you can refer to VuePress documentation on the actual implementation here: https://vuepress.vuejs.org/guide/.
+   ```sh
+   npm run dev
+   ```
+
+Once setup is complete, you can refer to VuePress documentation on the actual implementation here: <https://vuepress.vuejs.org/guide/>.
 
-### Git/GitHub workflow
+## Git/GitHub workflow
 
 This is our preferred process for opening a PR on GitHub:
 

Cargo.lock

@@ -31,7 +31,7 @@ tls-vendored = ["native-tls/vendored"]
 clap = "2.33.1"
 ansi_term = "0.12.1"
 dirs-next = "1.0.1"
-git2 = { version = "0.13.8", default-features = false, features = [] }
+git2 = { version = "0.13.8", default-features = false }
 toml = { version = "0.5.6", features = ["preserve_order"] }
 serde_json = "1.0.57"
 rayon = "1.3.1"
@@ -64,17 +64,19 @@ attohttpc = { version = "0.15.0", optional = true, default-features = false, fea
 native-tls = { version = "0.2", optional = true }
 
 [target.'cfg(windows)'.dependencies]
-winapi = { version = "0.3", features = ["winuser", "securitybaseapi", "processthreadsapi", "handleapi", "impl-default"]}
+winapi = { version = "0.3", features = [
+  "winuser",
+  "securitybaseapi",
+  "processthreadsapi",
+  "handleapi",
+  "impl-default",
+] }
 
 [target.'cfg(not(windows))'.dependencies]
 nix = "0.18.0"
 
 [dev-dependencies]
 tempfile = "3.1.0"
-# More realiable than std::fs version on Windows
-# For removing temporary directories manually when needed
-# This is what tempfile uses to delete temporary directories
-remove_dir_all = "0.5.3"
 
 [profile.release]
 codegen-units = 1

Cargo.toml

@@ -1,5 +1,6 @@
 use crate::utils::exec_cmd;
 
+use clap::crate_version;
 use std::fs;
 use std::path::PathBuf;
 
@@ -254,5 +255,6 @@ mod tests {
 
         let config_path = get_config_path("bash");
         assert_eq!("/test/home/.bashrc", config_path.unwrap().to_str().unwrap());
+        env::remove_var("HOME");
     }
 }

src/bug_report.rs

@@ -7,6 +7,7 @@ use git2::{ErrorCode::UnbornBranch, Repository, RepositoryState};
 use once_cell::sync::OnceCell;
 use std::collections::{HashMap, HashSet};
 use std::env;
+use std::ffi::OsString;
 use std::fs;
 use std::path::{Path, PathBuf};
 use std::string::String;
@@ -33,6 +34,9 @@ pub struct Context<'a> {
 
     /// The shell the user is assumed to be running
     pub shell: Shell,
+
+    /// A HashMap of environment variable mocks
+    pub env: HashMap<&'a str, String>,
 }
 
 impl<'a> Context<'a> {
@@ -82,11 +86,30 @@ impl<'a> Context<'a> {
             dir_contents: OnceCell::new(),
             repo: OnceCell::new(),
             shell,
+            env: HashMap::new(),
+        }
+    }
+
+    // Retrives a environment variable from the os or from a table if in testing mode
+    pub fn get_env<K: AsRef<str>>(&self, key: K) -> Option<String> {
+        if cfg!(test) {
+            self.env.get(key.as_ref()).map(|val| val.to_string())
+        } else {
+            env::var(key.as_ref()).ok()
+        }
+    }
+
+    // Retrives a environment variable from the os or from a table if in testing mode (os version)
+    pub fn get_env_os<K: AsRef<str>>(&self, key: K) -> Option<OsString> {
+        if cfg!(test) {
+            self.env.get(key.as_ref()).map(OsString::from)
+        } else {
+            env::var_os(key.as_ref())
         }
     }
 
     /// Convert a `~` in a path to the home directory
-    fn expand_tilde(dir: PathBuf) -> PathBuf {
+    pub fn expand_tilde(dir: PathBuf) -> PathBuf {
         if dir.starts_with("~") {
             let without_home = dir.strip_prefix("~").unwrap();
             return dirs_next::home_dir().unwrap().join(without_home);
@@ -165,7 +188,7 @@ impl<'a> Context<'a> {
     }
 
     fn get_shell() -> Shell {
-        let shell = std::env::var("STARSHIP_SHELL").unwrap_or_default();
+        let shell = env::var("STARSHIP_SHELL").unwrap_or_default();
         match shell.as_str() {
             "bash" => Shell::Bash,
             "fish" => Shell::Fish,
@@ -310,7 +333,7 @@ impl<'a> ScanDir<'a> {
         self
     }
 
-    /// based on the current Pathbuf check to see
+    /// based on the current PathBuf check to see
     /// if any of this criteria match or exist and returning a boolean
     pub fn is_match(&self) -> bool {
         self.dir_contents.has_any_extension(self.extensions)

src/context.rs

@@ -1,4 +1,5 @@
 use pest::{error::Error, iterators::Pair, Parser};
+use pest_derive::*;
 
 use super::model::*;
 

src/formatter/parser.rs

@@ -1,6 +1,3 @@
-#[macro_use]
-extern crate pest_derive;
-
 // Lib is present to allow for benchmarking
 pub mod config;
 pub mod configs;
@@ -11,3 +8,6 @@ pub mod modules;
 pub mod print;
 pub mod segment;
 mod utils;
+
+#[cfg(test)]
+mod test;

src/lib.rs

@@ -1,11 +1,7 @@
+use clap::{crate_authors, crate_version};
 use std::io;
 use std::time::SystemTime;
 
-#[macro_use]
-extern crate clap;
-#[macro_use]
-extern crate pest_derive;
-
 mod bug_report;
 mod config;
 mod configs;
@@ -19,6 +15,9 @@ mod print;
 mod segment;
 mod utils;
 
+#[cfg(test)]
+mod test;
+
 use crate::module::ALL_MODULES;
 use clap::{App, AppSettings, Arg, Shell, SubCommand};