From 89fb8188b7acf4f55974c920e94133e6619df832 Mon Sep 17 00:00:00 2001 From: "Elf M. Sternberg" Date: Tue, 21 Mar 2023 17:52:44 -0700 Subject: [PATCH] Pre-commit checks and test refactorings. Re-reading the text, I made a number of changes. The first is that, while it is nice that Rust allows us to have unit tests in the file whose functionality we're testing, it's also nice to have the tests somewhere separate, and to have the tests be a little more modular. In the `./tests` folder, you can now see the same `health_check` test as the original, but in an isolated and cleaned-up form. Most importantly, the server startup code is now in its own function, with a correct return type that includes a handle to the spawned thread and the address on which that server is listening; tests can be run in parallel on many different ports and a lot of code duplication is eliminated. ``` rust type NullHandle = JoinHandle<()>; async fn spawn_server() -> (SocketAddr, NullHandle) { let listener = TcpListener::bind("127.0.0.1:0".parse::().unwrap()).unwrap(); let addr = listener.local_addr().unwrap(); let handle: NullHandle = tokio::spawn(async move { axum::Server::from_tcp(listener) .unwrap() .serve(app().into_make_service()) .await .unwrap(); }); (addr, handle) } ``` It is also possible now to add new tests in a straightforward manner. The Hyper API is not that much different from the Actix request API, and the Axum extractors seem to be straightforward. I suspect that what I'm looking at here with the handle is the idea that, when it goes out of scope, it calls a d In the introduction I said I was going to be neglecting CI/CD, since I'm a solo developer. That's true, but I do like my guardrails. I like not being able to commit garbage to the repository. So I'm going to add some checks, using [Pre-Commit](https://pre-commit.com/). Pre-Commit is a Python program, so we'll start by installing it. I'm using a local Python environment kickstarted with [Pyenv](https://github.com/pyenv/pyenv). ``` sh $ pip install pre-commit ``` And inside your project, in the project root, you hook it up with the following commands: ``` sh $ pre-commit install $ pre-commit sample-config > .pre-commit-config.yaml ``` I'm going with the default from the rust pre-commit collection, so my `.pre-commit-config.yaml` file looks like this: ``` yaml repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v3.1.0 hooks: - id: check-byte-order-marker - id: check-case-conflict - id: check-merge-conflict - id: check-symlinks - id: check-yaml - id: end-of-file-fixer - id: mixed-line-ending - id: trailing-whitespace - repo: https://github.com/pre-commit/pre-commit rev: v2.5.1 hooks: - id: validate_manifest - repo: https://github.com/doublify/pre-commit-rust rev: master hooks: - id: fmt - id: cargo-check - id: clippy ``` ... and with that, every time I try to commit my code, it will not let me until these tests pass. And I *like* that level of discipline. This is low-level validation; it won't catch if I put addition where I meant subtraction, or if I have a comparison going in the wrong direction, but at least the basics are handled and, more importantly, the formatting and styling is consistent throughout all of my code. --- .markdownlint.yaml | 3 + .pre-commit-config.yaml | 29 ++++++ .pre-commit-hooks.yaml | 23 +++++ Cargo.lock | 15 +++ Cargo.toml | 5 + docs/01-installing-rust.md | 12 +-- docs/02-testing-hello-world.md | 21 ++-- docs/03-refactored-tests-and-pre-commits.md | 100 ++++++++++++++++++++ docs/_index.md | 1 - src/lib.rs | 10 +- src/user.rs | 13 +++ tests/health_check.rs | 95 +++++++++++++++++++ 12 files changed, 302 insertions(+), 25 deletions(-) create mode 100644 .markdownlint.yaml create mode 100644 .pre-commit-config.yaml create mode 100644 .pre-commit-hooks.yaml create mode 100644 docs/03-refactored-tests-and-pre-commits.md create mode 100644 src/user.rs create mode 100644 tests/health_check.rs diff --git a/.markdownlint.yaml b/.markdownlint.yaml new file mode 100644 index 0000000..2960864 --- /dev/null +++ b/.markdownlint.yaml @@ -0,0 +1,3 @@ +--- +commands-show-output: false +no-inline-html: false diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..977a3a5 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,29 @@ +--- +# See https://pre-commit.com for more information +# See https://pre-commit.com/hooks.html for more hooks +repos: +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v3.1.0 + hooks: + - id: check-byte-order-marker + - id: check-case-conflict + - id: check-merge-conflict + - id: check-symlinks + - id: check-yaml + - id: end-of-file-fixer + - id: mixed-line-ending + - id: trailing-whitespace +- repo: https://github.com/pre-commit/pre-commit + rev: v2.5.1 + hooks: + - id: validate_manifest +- repo: https://github.com/doublify/pre-commit-rust + rev: eeee35a8 + hooks: + - id: fmt + - id: cargo-check + - id: clippy +- repo: https://github.com/DavidAnson/markdownlint-cli2 + rev: v0.6.0 + hooks: + - id: markdownlint-cli2 diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml new file mode 100644 index 0000000..50bfb5e --- /dev/null +++ b/.pre-commit-hooks.yaml @@ -0,0 +1,23 @@ +--- +- id: fmt + name: fmt + description: Format files with cargo fmt. + entry: cargo fmt + language: system + types: [rust] + args: ["--"] +- id: cargo-check + name: cargo check + description: Check the package for errors. + entry: cargo check + language: system + types: [rust] + pass_filenames: false +- id: clippy + name: clippy + description: Lint rust sources + entry: cargo clippy + language: system + args: ["--", "-D", "warnings"] + types: [rust] + pass_filenames: false diff --git a/Cargo.lock b/Cargo.lock index c887f1d..e12caca 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -452,6 +452,20 @@ name = "serde" version = "1.0.158" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "771d4d9c4163ee138805e12c710dd365e4f44be8be0503cb1bb9eb989425d9c9" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.158" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e801c1712f48475582b7696ac71e0ca34ebb30e09338425384269d9717c62cad" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.4", +] [[package]] name = "serde_json" @@ -826,6 +840,7 @@ version = "0.1.0" dependencies = [ "axum", "hyper", + "serde", "tokio", "tower", "tracing", diff --git a/Cargo.toml b/Cargo.toml index 50cea7e..7a86efd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,8 +13,13 @@ name = "ztp" [dependencies] axum = "0.6.11" hyper = { version = "0.14.25", features = ["full"] } +serde = { version = "1.0.158", features = ["derive"] } tokio = { version = "1.26.0", features = ["full"] } tower = "0.4.13" tracing = "0.1.35" tracing-subscriber = "0.3.14" +[profile.release] +opt-level = 3 +lto = true +codegen-units = 1 diff --git a/docs/01-installing-rust.md b/docs/01-installing-rust.md index 73cb572..882ee4c 100644 --- a/docs/01-installing-rust.md +++ b/docs/01-installing-rust.md @@ -12,20 +12,20 @@ tool. It is one of those blind-trust-in-the-safety-of-the-toolchain things. For Linux and Mac users, the command is a shell script that installs to a user's local account: -``` +``` sh $ curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh ``` Once installed, you can install Rust itself: -``` +``` sh $ rustup install toolchain stable ``` You should now have Rust compiler and the Rust build and packaging tool, known as Cargo: -``` +``` sh $ rustc --version rustc 1.68.0 (2c8cc3432 2023-03-06) $ cargo --version @@ -34,8 +34,8 @@ cargo 1.68.0 (115f34552 2023-02-26) I also installed the following tools: -``` -$ rustup component add clippy rust-src rust-docs +``` sh +$ rustup component add clippy rust-src rust-docs $ cargo install rustfmt rust-analyzer ``` @@ -45,5 +45,3 @@ $ cargo install rustfmt rust-analyzer - rust-analyzer: For your IDE, rust-analyzer provides the LSP (Language Server Protocol) for Rust, giving you code completion, on-the-fly error definition, and other luxuries. - - diff --git a/docs/02-testing-hello-world.md b/docs/02-testing-hello-world.md index bed3378..d7522a0 100644 --- a/docs/02-testing-hello-world.md +++ b/docs/02-testing-hello-world.md @@ -16,12 +16,11 @@ Although the book is only two years old, it is already out-of-date with respect to some commands. `cargo add` is now provided by default. The following commands installed the tools I'll be using: -``` +``` sh cargo add --features tokio/full --features hyper/full tokio hyper \ axum tracing tracing-subscriber ``` - - axum: The web server framework for Tokio. - tokio: The Rust asynchronous runtime. Has single-threaded (select) and multi-threaded variants. @@ -39,7 +38,7 @@ converting data from one format to another. All of these go into `src/lib.rs`: -``` +``` rust async fn health_check() -> impl IntoResponse { (StatusCode::OK, ()) } @@ -65,7 +64,7 @@ E>`](https://doc.rust-lang.org/std/result/), especially one with an error. Router { Router::new() .route("/", get(anon_greet)) @@ -76,7 +75,7 @@ fn app() -> Router { We then define a function to *run* the core server: -``` +``` rust pub async fn run() { let addr = SocketAddr::from(([127, 0, 0, 1], 3000)); tracing::info!("listening on {}", addr); @@ -85,11 +84,11 @@ pub async fn run() { .await .unwrap() } -``` +``` And finally, in a file named `src/main.rs`, we instantiate the server: -``` +``` rust use ztp::run; #[tokio::main] @@ -104,7 +103,7 @@ between the library and the CLI program. In the project root's `Cargo.toml` file, the first three sections are needed to define these relationships: -``` +``` toml [package] name = "ztp" version = "0.1.0" @@ -127,13 +126,13 @@ project to have more than one package, called "crates," per project. This project should now be runnable. In one window, type: -``` +``` sh $ cargo run ``` And in another, type and see the replies: -``` +``` sh $ curl http://localhost:3000/ Hello, World! $ curl http://localhost:3000/Jim @@ -159,7 +158,7 @@ exercise its functions. We'll use Tokio's `spawn` function to create a new server, use hyper to request data from the server, and finally Rust's own native test asserts to check that we got what we expected. -``` +``` rust #[cfg(test)] mod tests { use super::*; diff --git a/docs/03-refactored-tests-and-pre-commits.md b/docs/03-refactored-tests-and-pre-commits.md new file mode 100644 index 0000000..192245e --- /dev/null +++ b/docs/03-refactored-tests-and-pre-commits.md @@ -0,0 +1,100 @@ ++++ +title = "Refactored Tests and Pre-Commits" +date = 2023-03-20T17:38:12Z +weight = 3 ++++ + +## Chapter 3.7 (Sort-of): A brief diversion + +Re-reading the text, I made a number of changes. The first is that, while it is +nice that Rust allows us to have unit tests in the file whose functionality +we're testing, it's also nice to have the tests somewhere separate, and to have +the tests be a little more modular. + +In the `./tests` folder, you can now see the same `health_check` test as the +original, but in an isolated and cleaned-up form. Most importantly, the server +startup code is now in its own function, with a correct return type that +includes a handle to the spawned thread and the address on which that server is +listening; tests can be run in parallel on many different ports and a lot of +code duplication is eliminated. + +``` rust +type NullHandle = JoinHandle<()>; + +async fn spawn_server() -> (SocketAddr, NullHandle) { + let listener = TcpListener::bind("127.0.0.1:0".parse::().unwrap()).unwrap(); + let addr = listener.local_addr().unwrap(); + + let handle: NullHandle = tokio::spawn(async move { + axum::Server::from_tcp(listener) + .unwrap() + .serve(app().into_make_service()) + .await + .unwrap(); + }); + + (addr, handle) +} +``` + +It is also possible now to add new tests in a straightforward manner. The +Hyper API is not that much different from the Actix request API, and the Axum +extractors seem to be straightforward. I suspect that what I'm looking at here +with the handle is the idea that, when it goes out of scope, it calls a d + +## Adding some checks + +In the introduction I said I was going to be neglecting CI/CD, since I'm a solo +developer. That's true, but I do like my guardrails. I like not being able to +commit garbage to the repository. So I'm going to add some checks, using +[Pre-Commit](https://pre-commit.com/). + +Pre-Commit is a Python program, so we'll start by installing it. I'm using a +local Python environment kickstarted with +[Pyenv](https://github.com/pyenv/pyenv). + +``` sh +$ pip install pre-commit +``` + +And inside your project, in the project root, you hook it up with the following commands: + +``` sh +$ pre-commit install +$ pre-commit sample-config > .pre-commit-config.yaml +``` + +I'm going with the default from the rust pre-commit collection, so my +`.pre-commit-config.yaml` file looks like this: + +``` yaml +repos: +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v3.1.0 + hooks: + - id: check-byte-order-marker + - id: check-case-conflict + - id: check-merge-conflict + - id: check-symlinks + - id: check-yaml + - id: end-of-file-fixer + - id: mixed-line-ending + - id: trailing-whitespace +- repo: https://github.com/pre-commit/pre-commit + rev: v2.5.1 + hooks: + - id: validate_manifest +- repo: https://github.com/doublify/pre-commit-rust + rev: master + hooks: + - id: fmt + - id: cargo-check + - id: clippy +``` + +... and with that, every time I try to commit my code, it will not let me until +these tests pass. And I *like* that level of discipline. This is low-level +validation; it won't catch if I put addition where I meant subtraction, or if I +have a comparison going in the wrong direction, but at least the basics are +handled and, more importantly, the formatting and styling is consistent +throughout all of my code. diff --git a/docs/_index.md b/docs/_index.md index 3d53518..9688db6 100644 --- a/docs/_index.md +++ b/docs/_index.md @@ -17,4 +17,3 @@ book. Those changes include: - Neglecting CI/CD, since I'm a sole developer - Developing entirely in a laptop environment - Using Axum instead of Actix-Web for the server framework - diff --git a/src/lib.rs b/src/lib.rs index eb90f99..61c9bcb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,10 +1,8 @@ use axum::{extract::Path, http::StatusCode, response::IntoResponse, routing::get, Router}; - use std::net::SocketAddr; -async fn anon_greet() -> &'static str { - "Hello World!\n" -} +mod user; +use user::index; async fn greet(Path(name): Path) -> impl IntoResponse { let greeting = String::from("He's dead, ") + name.as_str(); @@ -16,9 +14,9 @@ async fn health_check() -> impl IntoResponse { (StatusCode::OK, ()) } -fn app() -> Router { +pub fn app() -> Router { Router::new() - .route("/", get(anon_greet)) + .route("/", get(index)) .route("/:name", get(greet)) .route("/health_check", get(health_check)) } diff --git a/src/user.rs b/src/user.rs new file mode 100644 index 0000000..e5cbe5a --- /dev/null +++ b/src/user.rs @@ -0,0 +1,13 @@ +use axum::{http::StatusCode, response::IntoResponse, Form}; + +#[derive(serde::Deserialize)] +pub struct FormData { + username: String, +} + +pub async fn index(payload: Option>) -> impl IntoResponse { + let username = payload.map_or("World !".to_string(), move |index| -> String { + String::from(&(index.username)) + }); + (StatusCode::OK, format!("Hello, {}", &username)) +} diff --git a/tests/health_check.rs b/tests/health_check.rs new file mode 100644 index 0000000..ace435a --- /dev/null +++ b/tests/health_check.rs @@ -0,0 +1,95 @@ +use axum::{ + body::Body, + http::{Request, StatusCode}, +}; +use std::net::{SocketAddr, TcpListener}; +use tokio::task::JoinHandle; +use ztp::*; + +type NullHandle = JoinHandle<()>; + +async fn spawn_server() -> (SocketAddr, NullHandle) { + let listener = TcpListener::bind("127.0.0.1:0".parse::().unwrap()).unwrap(); + let addr = listener.local_addr().unwrap(); + + let handle: NullHandle = tokio::spawn(async move { + axum::Server::from_tcp(listener) + .unwrap() + .serve(app().into_make_service()) + .await + .unwrap(); + }); + + (addr, handle) +} + +#[tokio::test] +async fn the_real_deal() { + let (addr, _server_handle) = spawn_server().await; + + let response = hyper::Client::new() + .request( + Request::builder() + .uri(format!("http://{}/", addr)) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + let body = hyper::body::to_bytes(response.into_body()).await.unwrap(); + assert_eq!(&body[..], b"Hello World!\n"); +} + +#[tokio::test] +async fn valid_subscription() { + let (addr, _server_handle) = spawn_server().await; + let body = "name=le%20guin&email=ursula_le_guin%40gmail.com"; + + let response = hyper::Client::new() + .request( + Request::builder() + .method("POST") + .uri(format!("http://{}/subscriptions", addr)) + .body(Body::from(body)) + .unwrap(), + ) + .await + .expect("Failed to execute request."); + + // Assert + assert_eq!(200, response.status().as_u16()); +} + +#[tokio::test] +async fn subscribe_returns_400_on_missing_data() { + let (addr, _server_handle) = spawn_server().await; + + let test_cases = vec![ + ("name=le%20guin", "missing the email"), + ("email=ursula_le_guin%40gmail.com", "missing the name"), + ("", "missing both name and email"), + ]; + + for (invalid_body, error_message) in test_cases { + let response = hyper::Client::new() + .request( + Request::builder() + .method("POST") + .uri(format!("http://{}/subscriptions", addr)) + .body(Body::from(invalid_body)) + .unwrap(), + ) + .await + .expect("Failed to execute request."); + + // TODO This should be 400 "Bad Request" + assert_eq!( + 405, + response.status().as_u16(), + // Additional customised error message on test failure + "The API did not fail with 400 Bad Request when the payload was {}.", + error_message + ); + } +}