diff options
author | Tilmann Meyer <allescrafterx@gmail.com> | 2020-08-07 21:13:12 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-08-07 15:13:12 -0400 |
commit | 88b603be38aa623705e56e572eb174ae4d94107c (patch) | |
tree | 05033ec2adf013a55d3836a3f739e856880b5055 | |
parent | 8b0f589486e617c4fd52d870839a94d6e78f7379 (diff) |
test: introduce env variable mocking (#1490)
87 files changed, 4619 insertions, 4688 deletions
diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index 87daad63d..dfc2872dd 100644 --- a/.github/workflows/workflow.yml +++ b/.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' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index dd470e8f6..863885287 100644 --- a/CONTRIBUTING.md +++ b/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: diff --git a/Cargo.lock b/Cargo.lock index 03bb1918f..86f00fbc0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1104,7 +1104,6 @@ dependencies = [ "quick-xml", "rayon", "regex", - "remove_dir_all", "serde_json", "starship_module_config_derive", "sysinfo", diff --git a/Cargo.toml b/Cargo.toml index 34f26c658..e2555deb2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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 diff --git a/src/bug_report.rs b/src/bug_report.rs index 6735f2442..d9104ab1c 100644 --- a/src/bug_report.rs +++ b/src/bug_report.rs @@ -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"); } } diff --git a/src/context.rs b/src/context.rs index 9076120cf..40b547a77 100644 --- a/src/context.rs +++ b/src/context.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 o |