From 9a6e51d10ea9c816a506f65dba8ba9bc0c7a9143 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Wed, 10 Apr 2024 11:41:32 +0100 Subject: [PATCH] chore: fix linting warnigns and add testing workflow --- .cargo/config.toml | 26 +++++++ .github/CODEOWNERS | 1 + .github/dependabot.yaml | 19 +++++ .github/workflows/testing.yaml | 126 ++++++++++++++++++++++++++++++++ .gitignore | 8 ++ .vscode/extensions.json | 7 ++ .vscode/settings.json | 35 +++++++++ README.md | 4 +- docs/custom-mocks-in-rust.md | 54 +++++++------- docs/testing-apis-in-rust.md | 4 +- src/bin/custom-mocks-in-rust.rs | 9 ++- src/bin/testing-apis-in-rust.rs | 5 +- src/example01/events.rs | 5 +- src/example01/handlers.rs | 12 ++- src/example01/tracker.rs | 13 ++-- src/example02/api.rs | 6 +- tests/testing-apis-in-rust.rs | 2 +- 17 files changed, 287 insertions(+), 49 deletions(-) create mode 100644 .cargo/config.toml create mode 100644 .github/CODEOWNERS create mode 100644 .github/dependabot.yaml create mode 100644 .github/workflows/testing.yaml create mode 100644 .vscode/extensions.json create mode 100644 .vscode/settings.json diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..34d6230 --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,26 @@ +[alias] +cov = "llvm-cov" +cov-lcov = "llvm-cov --lcov --output-path=./.coverage/lcov.info" +cov-html = "llvm-cov --html" +time = "build --timings --all-targets" + +[build] +rustflags = [ + "-D", + "warnings", + "-D", + "future-incompatible", + "-D", + "let-underscore", + "-D", + "nonstandard-style", + "-D", + "rust-2018-compatibility", + "-D", + "rust-2018-idioms", + "-D", + "rust-2021-compatibility", + "-D", + "unused", +] + diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS new file mode 100644 index 0000000..d33c180 --- /dev/null +++ b/.github/CODEOWNERS @@ -0,0 +1 @@ +@josecelano diff --git a/.github/dependabot.yaml b/.github/dependabot.yaml new file mode 100644 index 0000000..becfbc1 --- /dev/null +++ b/.github/dependabot.yaml @@ -0,0 +1,19 @@ +version: 2 +updates: + - package-ecosystem: github-actions + directory: / + schedule: + interval: daily + target-branch: "develop" + labels: + - "Continuous Integration" + - "Dependencies" + + - package-ecosystem: cargo + directory: / + schedule: + interval: daily + target-branch: "develop" + labels: + - "Build | Project System" + - "Dependencies" diff --git a/.github/workflows/testing.yaml b/.github/workflows/testing.yaml new file mode 100644 index 0000000..4c2e45a --- /dev/null +++ b/.github/workflows/testing.yaml @@ -0,0 +1,126 @@ +name: Testing + +on: + push: + pull_request: + +env: + CARGO_TERM_COLOR: always + +jobs: + format: + name: Formatting + runs-on: ubuntu-latest + + steps: + - id: checkout + name: Checkout Repository + uses: actions/checkout@v4 + + - id: setup + name: Setup Toolchain + uses: dtolnay/rust-toolchain@stable + with: + toolchain: nightly + components: rustfmt + + - id: cache + name: Enable Workflow Cache + uses: Swatinem/rust-cache@v2 + + - id: format + name: Run Formatting-Checks + run: cargo fmt --check + + check: + name: Static Analysis + runs-on: ubuntu-latest + needs: format + + strategy: + matrix: + toolchain: [nightly, stable] + + steps: + - id: checkout + name: Checkout Repository + uses: actions/checkout@v4 + + - id: setup + name: Setup Toolchain + uses: dtolnay/rust-toolchain@stable + with: + toolchain: ${{ matrix.toolchain }} + components: clippy + + - id: cache + name: Enable Workflow Cache + uses: Swatinem/rust-cache@v2 + + - id: tools + name: Install Tools + uses: taiki-e/install-action@v2 + with: + tool: cargo-machete + + - id: check + name: Run Build Checks + run: cargo check --tests --benches --examples --workspace --all-targets --all-features + + - id: lint + name: Run Lint Checks + run: cargo clippy --tests --benches --examples --workspace --all-targets --all-features -- -D clippy::correctness -D clippy::suspicious -D clippy::complexity -D clippy::perf -D clippy::style -D clippy::pedantic + + - id: docs + name: Lint Documentation + env: + RUSTDOCFLAGS: "-D warnings" + run: cargo doc --no-deps --bins --examples --workspace --all-features + + - id: clean + name: Clean Build Directory + run: cargo clean + + - id: deps + name: Check Unused Dependencies + run: cargo machete + + + unit: + name: Units + runs-on: ubuntu-latest + needs: check + + strategy: + matrix: + toolchain: [nightly, stable] + + steps: + - id: checkout + name: Checkout Repository + uses: actions/checkout@v4 + + - id: setup + name: Setup Toolchain + uses: dtolnay/rust-toolchain@stable + with: + toolchain: ${{ matrix.toolchain }} + components: llvm-tools-preview + + - id: cache + name: Enable Job Cache + uses: Swatinem/rust-cache@v2 + + - id: tools + name: Install Tools + uses: taiki-e/install-action@v2 + with: + tool: cargo-llvm-cov, cargo-nextest + + - id: test-docs + name: Run Documentation Tests + run: cargo test --doc + + - id: test + name: Run Unit Tests + run: cargo test --tests --benches --examples --workspace --all-targets --all-features diff --git a/.gitignore b/.gitignore index ea8c4bf..c552776 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,9 @@ +.env +**/*.rs.bk +/.coverage/ +/.idea/ +/.vscode/launch.json +/flamegraph.svg /target +callgrind.out +perf.data* diff --git a/.vscode/extensions.json b/.vscode/extensions.json new file mode 100644 index 0000000..934a43e --- /dev/null +++ b/.vscode/extensions.json @@ -0,0 +1,7 @@ +{ + "recommendations": [ + "streetsidesoftware.code-spell-checker", + "rust-lang.rust-analyzer", + "tamasfe.even-better-toml" + ] +} \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..caa48dd --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,35 @@ +{ + "[rust]": { + "editor.formatOnSave": true + }, + "[ignore]": { "rust-analyzer.cargo.extraEnv" : { + "RUSTFLAGS": "-Z profile -C codegen-units=1 -C inline-threshold=0 -C link-dead-code -C overflow-checks=off -C panic=abort -Z panic_abort_tests", + "RUSTDOCFLAGS": "-Z profile -C codegen-units=1 -C inline-threshold=0 -C link-dead-code -C overflow-checks=off -C panic=abort -Z panic_abort_tests", + "CARGO_INCREMENTAL": "0", + "RUST_BACKTRACE": "1" + }}, + "rust-analyzer.checkOnSave": true, + "rust-analyzer.check.command": "clippy", + "rust-analyzer.check.allTargets": true, + "rust-analyzer.check.extraArgs": [ + "--", + "-D", + "clippy::correctness", + "-D", + "clippy::suspicious", + "-W", + "clippy::complexity", + "-W", + "clippy::perf", + "-W", + "clippy::style", + "-W", + "clippy::pedantic" + ], + "evenBetterToml.formatter.allowedBlankLines": 1, + "evenBetterToml.formatter.columnWidth": 130, + "evenBetterToml.formatter.trailingNewline": true, + "evenBetterToml.formatter.reorderKeys": true, + "evenBetterToml.formatter.reorderArrays": true, + +} \ No newline at end of file diff --git a/README.md b/README.md index 5f1632d..db07c5b 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # Testing in Rust +[![Testing](https://github.com/nautilus-cyberneering/testing-in-rust/actions/workflows/testing.yaml/badge.svg)](https://github.com/nautilus-cyberneering/testing-in-rust/actions/workflows/testing.yaml) + A collection of articles about testing in Rust. - [Custom mocks in Rust](./docs/custom-mocks-in-rust.md). @@ -8,7 +10,7 @@ A collection of articles about testing in Rust. ## Links - ["Unit Testing" book](https://www.manning.com/books/unit-testing) by [Vladimir Khorikov](https://github.com/vkhorikov). -- [Mockall](https://github.com/asomers/mockall). A powerful mock object library for Rust(). +- [Mockall](https://github.com/asomers/mockall). A powerful mock object library for Rust. - [Rust book. "A Use Case for Interior Mutability: Mock Objects"](https://doc.rust-lang.org/book/ch15-05-interior-mutability.html#a-use-case-for-interior-mutability-mock-objects) by [Steve Klabnik](https://steveklabnik.com/) and [Carol Nichols](http://carol-nichols.com/). - [Mock Objects (manually) in Rust](https://paytonrules.com/post/mock-objects-in-rust/) by [Eric Smith](https://github.com/paytonrules). - [A guide to mocking in Rust using Mockall](https://blog.logrocket.com/guide-mocking-rust-mockall/) by Manish Shivanandhan. diff --git a/docs/custom-mocks-in-rust.md b/docs/custom-mocks-in-rust.md index f3c6d1e..f18992a 100644 --- a/docs/custom-mocks-in-rust.md +++ b/docs/custom-mocks-in-rust.md @@ -72,7 +72,7 @@ fn the_tracker_should_send_a_connect_event_after_connecting() { // Test using a custom mock for the TrackerEventSender let event_sender = Rc::new(TrackerEventSenderMock::new()); - let tracker = Arc::new(Tracker::new(event_sender.clone())); + let tracker = Rc::new(Tracker::new(event_sender.clone())); tracker.connect(); @@ -88,7 +88,7 @@ self.sent_event = Some(event); Since the `self` reference is not mutable, you can not change the `sent_event` value. -I needed to learn how to implement it, and [Cameron](https://github.com/da2ce7) pointed me to the solution. The not mutable `self` reference does not allow you to change the attributes in the struct, but it's not recursive. Rust has some types that allow you to change the interior mutability. +I needed to learn how to implement it, and [Cameron](https://github.com/da2ce7) pointed me to the solution. The immutable `self` reference does not allow you to change the attributes in the struct, but it's not recursive. Rust has some types that allow you to change the interior mutability. Rust has a pattern called the ["Interior Mutability Pattern"](https://doc.rust-lang.org/book/ch15-05-interior-mutability.html#refcellt-and-the-interior-mutability-pattern). @@ -111,39 +111,39 @@ From the Rust book, you can see the different types to "bend" Rust rules: The final test was like this: ```rust - #[derive(Clone)] - struct TrackerEventSenderMock { - pub sent_event: RefCell>, - } +#[derive(Clone)] +struct TrackerEventSenderMock { + pub sent_event: RefCell>, +} - impl TrackerEventSenderMock { - pub fn new() -> Self { - Self { - sent_event: RefCell::new(None), - } +impl TrackerEventSenderMock { + pub fn new() -> Self { + Self { + sent_event: RefCell::new(None), } } +} - impl EventSender for TrackerEventSenderMock { - fn send_event(&self, event: Event) -> Result<(), Box> { - *self.sent_event.borrow_mut() = Some(event); +impl EventSender for TrackerEventSenderMock { + fn send_event(&self, event: Event) -> Result<(), Box> { + *self.sent_event.borrow_mut() = Some(event); - // We return the expected value - Ok(()) - } + // We return the expected value + Ok(()) } +} - #[test] - fn the_tracker_should_send_a_connect_event_after_connecting() { - // Test using a custom mock for the TrackerEventSender +#[test] +fn the_tracker_should_send_a_connect_event_after_connecting() { + // Test using a custom mock for the TrackerEventSender - let event_sender = Rc::new(TrackerEventSenderMock::new()); - let tracker = Arc::new(Tracker::new(event_sender.clone())); + let event_sender = Rc::new(TrackerEventSenderMock::new()); + let tracker = Rc::new(Tracker::new(event_sender.clone())); - tracker.connect().unwrap(); + tracker.connect().unwrap(); - assert_eq!(event_sender.sent_event.borrow().unwrap(), Event::Connect); - } + assert_eq!(event_sender.sent_event.borrow().unwrap(), Event::Connect); +} ``` ## Conclusion @@ -156,9 +156,9 @@ I finally decided to use `mockall` (the mocking framework) for a few reasons: 2. The `mockall` readability is better because of its fluent style. 3. `mockall` allows you to be more precise, for example checking also the number of calls. -You can read the final solution using [mockall](https://docs.rs/mockall/latest/mockall/) [here](https://github.com/torrust/torrust-tracker/blob/develop/src/udp/handlers.rs#L426-L463): +You can read the final solution using [mockall](https://docs.rs/mockall/latest/mockall/) [here](https://github.com/torrust/torrust-tracker/blob/af52045436644ba2ba3c43195a0c1b6b0bfdbd42/src/servers/udp/handlers.rs#L490-L527): - + ```rust #[tokio::test] diff --git a/docs/testing-apis-in-rust.md b/docs/testing-apis-in-rust.md index 18e7d77..145a47b 100644 --- a/docs/testing-apis-in-rust.md +++ b/docs/testing-apis-in-rust.md @@ -64,7 +64,7 @@ impl ApiServerStarter { } ``` -That was the first version, and we changed but before explaining the latest version, we want to explain why we needed to change it. +That was the first version, and we changed it but before explaining the latest version, we want to explain why we needed to change it. ## ConnectionRefused error @@ -103,7 +103,7 @@ pub async fn start(&mut self, addr: SocketAddr) { } ``` -That worked was we were not happy with adding a random sleep time. Because that: +That worked but we were not happy with adding a random sleep time. Because that: - Make tests slower. - Could make the test fail if, for some reason, the web server takes more than 100 milliseconds to be ready. diff --git a/src/bin/custom-mocks-in-rust.rs b/src/bin/custom-mocks-in-rust.rs index 6bee0e3..d1d0249 100644 --- a/src/bin/custom-mocks-in-rust.rs +++ b/src/bin/custom-mocks-in-rust.rs @@ -1,6 +1,7 @@ +//! ```text //! cargo run --bin custom-mocks-in-rust - -use std::{rc::Rc, sync::Arc}; +//! ``` +use std::rc::Rc; use testing_in_rust::example01::{ events::TrackerEventSender, handlers::handle_connect, tracker::Tracker, @@ -8,7 +9,7 @@ use testing_in_rust::example01::{ fn main() { let event_sender = Rc::new(TrackerEventSender {}); - let tracker = Arc::new(Tracker::new(event_sender)); + let tracker = Rc::new(Tracker::new(event_sender)); - handle_connect(tracker) + handle_connect(&tracker); } diff --git a/src/bin/testing-apis-in-rust.rs b/src/bin/testing-apis-in-rust.rs index 8e4d2b8..fc3aa4b 100644 --- a/src/bin/testing-apis-in-rust.rs +++ b/src/bin/testing-apis-in-rust.rs @@ -1,5 +1,6 @@ +//! ```text //! cargo run --bin testing-apis-in-rust -//! +//! ```` use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use testing_in_rust::example02::api::start_server; @@ -8,5 +9,5 @@ use testing_in_rust::example02::api::start_server; async fn main() { let bind_address = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 3030); - start_server(bind_address).await + start_server(bind_address).await; } diff --git a/src/example01/events.rs b/src/example01/events.rs index 42077f2..30133ac 100644 --- a/src/example01/events.rs +++ b/src/example01/events.rs @@ -8,6 +8,9 @@ pub enum Event { } pub trait EventSender { + /// # Errors + /// + /// Will return an error if the event can't be sent. fn send_event(&self, event: Event) -> Result<(), Box>; } @@ -16,7 +19,7 @@ pub struct TrackerEventSender {} impl EventSender for TrackerEventSender { fn send_event(&self, event: Event) -> Result<(), Box> { - println!("Event::{:?} sent.", event); + println!("Event::{event:?} sent."); Ok(()) } } diff --git a/src/example01/handlers.rs b/src/example01/handlers.rs index d757b93..f258669 100644 --- a/src/example01/handlers.rs +++ b/src/example01/handlers.rs @@ -1,8 +1,14 @@ -use std::sync::Arc; +use std::rc::Rc; use crate::example01::tracker::Tracker; /// Controller for the UDP connect request -pub fn handle_connect(tracker: Arc) { - tracker.connect().unwrap(); +/// +/// # Panics +/// +/// Will panic if the tracker can't handle the connect request. +pub fn handle_connect(tracker: &Rc) { + tracker + .connect() + .expect("Tracker should handle the connect request"); } diff --git a/src/example01/tracker.rs b/src/example01/tracker.rs index e2d8949..4822efc 100644 --- a/src/example01/tracker.rs +++ b/src/example01/tracker.rs @@ -2,7 +2,7 @@ use std::{error::Error, rc::Rc}; use crate::example01::events::{Event, EventSender}; -/// BitTorrent tracker +/// `BitTorrent` tracker pub struct Tracker { event_sender: Rc, } @@ -12,11 +12,14 @@ impl Tracker { Self { event_sender } } + /// # Errors + /// + /// Will return an error if `Connect` event cant' be sent. pub fn connect(&self) -> Result<(), Box> { println!("Tracker::connect."); // After connecting the tracker sends an event - self.event_sender.send_event(Event::Connect).unwrap(); + self.event_sender.send_event(Event::Connect)?; Ok(()) } @@ -24,7 +27,7 @@ impl Tracker { #[cfg(test)] mod tests { - use std::{cell::RefCell, error::Error, rc::Rc, sync::Arc}; + use std::{cell::RefCell, error::Error, rc::Rc}; use crate::example01::{ events::{Event, EventSender, TrackerEventSender}, @@ -35,7 +38,7 @@ mod tests { fn the_tracker_should_allow_connections() { // This is just a dummy test to show how we use the real struct instead of the mock let event_sender = Rc::new(TrackerEventSender {}); - let tracker = Arc::new(Tracker::new(event_sender)); + let tracker = Rc::new(Tracker::new(event_sender)); assert!(tracker.connect().is_ok()); } @@ -67,7 +70,7 @@ mod tests { // Test using a custom mock for the TrackerEventSender let event_sender = Rc::new(TrackerEventSenderMock::new()); - let tracker = Arc::new(Tracker::new(event_sender.clone())); + let tracker = Rc::new(Tracker::new(event_sender.clone())); tracker.connect().unwrap(); diff --git a/src/example02/api.rs b/src/example02/api.rs index 836a9c7..1b3a1db 100644 --- a/src/example02/api.rs +++ b/src/example02/api.rs @@ -1,11 +1,11 @@ use std::net::SocketAddr; -use colored::*; +use colored::Colorize; use warp::Filter; pub async fn start_server(addr: SocketAddr) { // GET /hello/warp => 200 OK with body "Hello, warp!" - let hello = warp::path!("hello" / String).map(|name| format!("Hello, {}!", name)); + let hello = warp::path!("hello" / String).map(|name| format!("Hello, {name}!")); let api_base_url = "http://127.0.0.1:3030/"; @@ -15,5 +15,5 @@ pub async fn start_server(addr: SocketAddr) { "hello/warp".yellow() ); - warp::serve(hello).run(addr).await + warp::serve(hello).run(addr).await; } diff --git a/tests/testing-apis-in-rust.rs b/tests/testing-apis-in-rust.rs index 305033b..f4f0aa4 100644 --- a/tests/testing-apis-in-rust.rs +++ b/tests/testing-apis-in-rust.rs @@ -10,7 +10,7 @@ async fn it_should_greeting_you() { start_server_and_wait_until_is_ready_to_accept_requests(bind_address).await; - let url = format!("http://{}/hello/{}", &bind_address, "warp"); + let url = format!("http://{}/hello/{}", &bind_address, "warp"); // DevSkim: ignore DS137138 let content = reqwest::get(url).await.unwrap().text().await.unwrap();