From ef723d440a873440daab4f8d88d5c0363810dea8 Mon Sep 17 00:00:00 2001 From: ASuciuX Date: Thu, 16 Jan 2025 18:16:02 +0200 Subject: [PATCH 1/2] Add: CI mutants output and update rust version - the version update to 1.81 is required to support mutation testing --- .github/workflows/pr-mutants.yml | 250 ++++++++++++++++++++++ .gitignore | 4 + Cargo.lock | 2 +- dockerfiles/components/ordhook.dockerfile | 2 +- rust-toolchain | 2 +- 5 files changed, 257 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/pr-mutants.yml diff --git a/.github/workflows/pr-mutants.yml b/.github/workflows/pr-mutants.yml new file mode 100644 index 00000000..a41120ad --- /dev/null +++ b/.github/workflows/pr-mutants.yml @@ -0,0 +1,250 @@ +name: PR Differences Mutants + +on: + pull_request: + types: + - opened + - reopened + - synchronize + - ready_for_review + paths: + - "**.rs" + workflow_dispatch: + +concurrency: + group: pr-differences-${{ github.head_ref || github.ref || github.run_id }} + # Always cancel duplicate jobs + cancel-in-progress: true + +jobs: + mutants: + name: Mutation Testing + runs-on: ubuntu-latest + steps: + # Cleanup Runner + - name: Cleanup Runner + id: runner_cleanup + uses: stacks-network/actions/cleanup/disk@main + + - name: Checkout repo + id: git_checkout + uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5 + with: + fetch-depth: 0 + + - name: Relative diff + id: relative_diff + run: | + git diff $(git merge-base origin/${{ github.base_ref || 'main' }} HEAD)..HEAD > git.diff + + - name: Install cargo-mutants + id: install_cargo_mutants + run: | + cargo install --version 24.11.2 cargo-mutants --locked # v24.11.2 + + - name: Install cargo-nextest + id: install_cargo_nextest + uses: taiki-e/install-action@2f990e9c484f0590cb76a07296e9677b417493e9 # v2.33.23 + with: + tool: nextest # Latest version + + - name: Update git diff + id: update_git_diff + run: | + input_file="git.diff" + temp_file="temp_diff_file.diff" + + # Check if the file exists and is not empty + if [ ! -s "$input_file" ]; then + echo "Diff file ($input_file) is missing or empty!" + exit 1 + fi + + # Remove all lines related to deleted files including the first 'diff --git' line + awk ' + /^diff --git/ { + diff_line = $0 + getline + if ($0 ~ /^deleted file mode/) { + in_deleted_file_block = 1 + } else { + if (diff_line != "") { + print diff_line + diff_line = "" + } + in_deleted_file_block = 0 + } + } + !in_deleted_file_block + ' "$input_file" > "$temp_file" && mv "$temp_file" "$input_file" + + # Remove 'diff --git' lines only when followed by 'similarity index', 'rename from', and 'rename to' + awk ' + /^diff --git/ { + diff_line = $0 + getline + if ($0 ~ /^similarity index/) { + getline + if ($0 ~ /^rename from/) { + getline + if ($0 ~ /^rename to/) { + next + } + } + } + print diff_line + } + { print } + ' "$input_file" > "$temp_file" && mv "$temp_file" "$input_file" + + - name: Run docker-compose + uses: hoverkraft-tech/compose-action@f1ca7fefe3627c2dab0ae1db43a106d82740245e # v2.0.2 + with: + compose-file: "./dockerfiles/docker-compose.dev.postgres.yml" + + - name: Run mutants + id: run_mutants + run: | + # Disable immediate exit on error + set +e + + cargo mutants --workspace --timeout-multiplier 1.5 --no-shuffle -vV --in-diff git.diff --output ./ --test-tool=nextest -- --all-targets --test-threads 1 + exit_code=$? + + # Create the folder only containing the outcomes (.txt files) and make a file containing the exit code of the command + mkdir mutants + echo "$exit_code" > ./mutants/exit_code.txt + mv ./mutants.out/*.txt mutants/ + + # Enable immediate exit on error again + set -e + + - name: Print mutants + id: print_tested_mutants + shell: bash + run: | + # Info for creating the link that paths to the specific mutation tested + server_url="${{ github.server_url }}" + organisation="${{ github.repository_owner }}" + repository="${{ github.event.repository.name }}" + commit="${{ github.sha }}" + + # Function to write to github step summary with specific info depending on the mutation category + write_section() { + local section_title=$1 + local file_name=$2 + + if [ -s "$file_name" ]; then + if [[ "$section_title" != "" ]]; then + echo "## $section_title" >> "$GITHUB_STEP_SUMMARY" + fi + + if [[ "$section_title" == "Missed:" ]]; then + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "What are missed mutants?" >> "$GITHUB_STEP_SUMMARY" + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "No test failed with this mutation applied, which seems to indicate a gap in test coverage. Or, it may be that the mutant is undistinguishable from the correct code. You may wish to add a better test, or mark that the function should be skipped." >> "$GITHUB_STEP_SUMMARY" + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + elif [[ "$section_title" == "Timeout:" ]]; then + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "What are timeout mutants?" >> "$GITHUB_STEP_SUMMARY" + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "The mutation caused the test suite to run for a long time, until it was eventually killed. You might want to investigate the cause and potentially mark the function to be skipped." >> "$GITHUB_STEP_SUMMARY" + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + elif [[ "$section_title" == "Unviable:" ]]; then + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "What are unviable mutants?" >> "$GITHUB_STEP_SUMMARY" + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "The attempted mutation doesn't compile. This is inconclusive about test coverage and no action is needed, unless you wish to test the specific function, in which case you may wish to add a 'Default::default()' implementation for the specific return type." >> "$GITHUB_STEP_SUMMARY" + echo "
" >> "$GITHUB_STEP_SUMMARY" + echo "" >> "$GITHUB_STEP_SUMMARY" + fi + + if [[ "$section_title" != "" ]]; then + awk -F':' '{gsub("%", "%%"); printf "- [ ] [%s](%s/%s/%s/blob/%s/%s#L%d)\n\n", $0, "'"$server_url"'", "'"$organisation"'", "'"$repository"'", "'"$commit"'", $1, $2-1}' "$file_name" >> "$GITHUB_STEP_SUMMARY" + else + awk -F':' '{gsub("%", "%%"); printf "- [x] [%s](%s/%s/%s/blob/%s/%s#L%d)\n\n", $0, "'"$server_url"'", "'"$organisation"'", "'"$repository"'", "'"$commit"'", $1, $2-1}' "$file_name" >> "$GITHUB_STEP_SUMMARY" + fi + + if [[ "$section_title" == "Missed:" ]]; then + echo "### To resolve this issue, consider one of the following options:" >> "$GITHUB_STEP_SUMMARY" + echo "- Modify or add tests including this function." >> "$GITHUB_STEP_SUMMARY" + echo "- If you are absolutely certain that this function should not undergo mutation testing, add '#[mutants::skip]' or '#[cfg_attr(test, mutants::skip)]' function header to skip it." >> "$GITHUB_STEP_SUMMARY" + elif [[ "$section_title" == "Timeout:" ]]; then + echo "### To resolve this issue, consider one of the following options:" >> "$GITHUB_STEP_SUMMARY" + echo "- Modify the tests that include this funcion." >> "$GITHUB_STEP_SUMMARY" + echo "- Add '#[mutants::skip]' or '#[cfg_attr(test, mutants::skip)]' function header to skip it." >> "$GITHUB_STEP_SUMMARY" + elif [[ "$section_title" == "Unviable:" ]]; then + echo "### To resolve this issue, consider one of the following options:" >> "$GITHUB_STEP_SUMMARY" + echo "- Create 'Default::default()' implementation for the specific structure." >> "$GITHUB_STEP_SUMMARY" + echo "- Add '#[mutants::skip]' or '#[cfg_attr(test, mutants::skip)]' function header to skip it." >> "$GITHUB_STEP_SUMMARY" + fi + + echo >> "$GITHUB_STEP_SUMMARY" + fi + } + + # Print uncaught (missed/timeout/unviable) mutants to summary + if [ -s ./mutants/missed.txt -o -s ./mutants/timeout.txt -o -s ./mutants/unviable.txt ]; then + echo "# Uncaught Mutants" >> "$GITHUB_STEP_SUMMARY" + echo "[Documentation - How to treat Mutants Output](https://github.com/stacks-network/actions/tree/main/stacks-core/mutation-testing#how-mutants-output-should-be-treated)" >> "$GITHUB_STEP_SUMMARY" + write_section "Missed:" "./mutants/missed.txt" + write_section "Timeout:" "./mutants/timeout.txt" + write_section "Unviable:" "./mutants/unviable.txt" + fi + + # Print caught mutants to summary + if [ -s ./mutants/caught.txt ]; then + echo "# Caught Mutants" >> "$GITHUB_STEP_SUMMARY" + write_section "" "./mutants/caught.txt" + fi + + # Get exit code from the file and match it + exit_code=$(<"mutants/exit_code.txt") + + yellow_bold="\033[1;33m" + reset="\033[0m" + summary_link_message="${yellow_bold}Click here for more information on how to fix:${reset} ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}#:~:text=Output%20Mutants%20summary" + + case $exit_code in + 0) + if [[ ! -f ./mutants/caught.txt && ! -f ./mutants/missed.txt && ! -f ./mutants/timeout.txt && ! -f ./mutants/unviable.txt ]]; then + echo "No mutants found to test!" + elif [[ -s ./mutants/unviable.txt ]]; then + echo -e "$summary_link_message" + echo "Found unviable mutants!" + exit 5 + else + echo "All new and updated functions are caught!" + fi + ;; + 1) + echo -e "$summary_link_message" + echo "Invalid command line arguments!" + exit $exit_code + ;; + 2) + echo -e "$summary_link_message" + echo "Found missed mutants!" + exit $exit_code + ;; + 3) + echo -e "$summary_link_message" + echo "Found timeout mutants!" + exit $exit_code + ;; + 4) + echo -e "$summary_link_message" + echo "Building the packages failed without any mutations!" + exit $exit_code + ;; + *) + echo -e "$summary_link_message" + echo "Unknown exit code: $exit_code" + exit $exit_code + ;; + esac + + exit 0 diff --git a/.gitignore b/.gitignore index 79e2d3fc..ad4b20f2 100644 --- a/.gitignore +++ b/.gitignore @@ -222,3 +222,7 @@ Cargo.lock !.yarn/versions *.node + +# Mutation Testing +mutants.out/ +mutants.out.old/ diff --git a/Cargo.lock b/Cargo.lock index f3a3f6a0..65c7ee5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1286,7 +1286,7 @@ checksum = "3bf679796c0322556351f287a51b49e48f7c4986e727b5dd78c972d30e2e16cc" dependencies = [ "proc-macro2", "quote", - "syn 2.0.41", + "syn 2.0.90", ] [[package]] diff --git a/dockerfiles/components/ordhook.dockerfile b/dockerfiles/components/ordhook.dockerfile index c312b758..c6d08536 100644 --- a/dockerfiles/components/ordhook.dockerfile +++ b/dockerfiles/components/ordhook.dockerfile @@ -7,7 +7,7 @@ ENV GIT_COMMIT=${GIT_COMMIT} WORKDIR /src RUN apt-get update && apt-get install -y ca-certificates pkg-config libssl-dev libclang-11-dev libunwind-dev libunwind8 curl gnupg -RUN rustup update 1.80.1 && rustup default 1.80.1 +RUN rustup update 1.81 && rustup default 1.81 RUN mkdir /out COPY ./Cargo.toml /src/Cargo.toml diff --git a/rust-toolchain b/rust-toolchain index a56a283d..4cef0b73 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1,2 +1,2 @@ [toolchain] -channel = "1.80.1" +channel = "1.81" From 0a4fb13be7c30bb3d9f49d313f3a9c366b1c0e37 Mon Sep 17 00:00:00 2001 From: ASuciuX Date: Thu, 16 Jan 2025 20:47:08 +0200 Subject: [PATCH 2/2] docs: add Contributing file --- .github/CONTRIBUTING.md | 131 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 .github/CONTRIBUTING.md diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md new file mode 100644 index 00000000..edd9f4b6 --- /dev/null +++ b/.github/CONTRIBUTING.md @@ -0,0 +1,131 @@ +# Contributing + +Thank you for considering contributing to this product! We welcome any contributions, whether it's bug fixes, new features, or improvements to the existing codebase. + +## Your First Pull Request + +Working on your first Pull Request? You can learn how from this free video series: + +[How to Contribute to an Open Source Project on GitHub](https://egghead.io/courses/how-to-contribute-to-an-open-source-project-on-github) + +To help you get familiar with our contribution process, we have a list of [good first issues](../../issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22) that contain bugs that have a relatively limited scope. This is a great place to get started. + +If you decide to fix an issue, please be sure to check the comment thread in case somebody is already working on a fix. If nobody is working on it at the moment, please leave a comment stating that you intend to work on it so other people don’t accidentally duplicate your effort. + +If somebody claims an issue but doesn’t follow up for more than two weeks, it’s fine to take it over but you should still leave a comment. **Issues won't be assigned to anyone outside the core team**. + +## How to run the repo test suite + +```bash +docker-compose -f dockerfiles/docker-compose.dev.postgres.yml up +cargo test --workspace +docker-compose -f dockerfiles/docker-compose.dev.postgres.yml down +``` + +## How CI Mutants Output should be treated + +1. **New Function Created in This PR:** + - **Knowledgeable:** + Ideally, write unit tests. + This takes more time initially but is beneficial long-term. + - **Not Knowledgeable:** + Create an issue to highlight the gap, check examples, using the `mutation-testing` label on GitHub: [Mutation Testing Issues](https://github.com/hirosystems/runehook/issues?q=is%3Aissue%20state%3Aopen%20label%3Amutation-testing). + +2. **Modified Function in This PR:** +Review the commit history to identify the developer who is familiar with the context of the function, create a new issue and tag him. + +### Types of Mutants + +1. **Caught:** +No action is needed as these represent well-tested functions. +2. **Missed:** +Add tests where coverage is lacking. +3. **Timeout:** +Use the skip flag for functions that include network requests/responses to avoid hang-ups due to alterations. +4. **Unviable:** +Implement defaults to enable running tests with these mutants. + + +### How to treat different types of mutants + +#### 1. Caught + +Caught mutants indicate functions that are well-tested, where mutations break the unit tests. +Aim to achieve this status. + +#### 2. Timeout + +Timeouts often occur in functions altered to include endless waits (e.g., altered HTTP requests/responses). Apply the `#[cfg_attr(test, mutants::skip)]` flag. +Look into the function that has the mutation creating a timeout and if it has http requests/responses, or any one or multiple child levels have requests/responses, add this flag like showcased in the below example. + ```rust + impl PeerNetwork { + #[cfg_attr(test, mutants::skip)] + /// Check that the sender is authenticated. + /// Returns Some(remote sender address) if so + /// Returns None otherwise + fn check_peer_authenticated(&self, event_id: usize) -> Option { + ``` + +#### 3. Missed +Missed mutants highlight that the function doesn’t have tests for specific cases. +eg. if the function returns a `bool` and the mutant replaces the function’s body with `true`, then a missed mutant reflects that the function is not tested for the `false` case as it passes all test cases by having this default `true` value. + +1. If you are the person creating the functions, most probably you are most adequate to create these tests. +2. If you are the person modifying the function, if you are aware of how it works it would be best to be added by you as in the long run it would create less context switching for others that are aware of the function’s tests. +3. If the context switching is worthy or you aren’t aware of the full context to add all the missing tests, than an issue should be created to highlight the problem and afterwards the tests be added or modified by someone else. [eg. issue format](https://github.com/stacks-network/stacks-core/issues/4872) + +#### 4. Unviable + +Unviable mutants show a need for a default value for the return type of the function. +This is needed in order for the function’s body to be replaced with this default value and run the test suite. +While this increases the chances of catching untested scenarios, it doesn’t mean it catches all of them. +[eg. issue format](https://github.com/stacks-network/stacks-core/issues/4867) +If a default implementation would not cover it, or it can’t be created for this structure for various reasons, it can be skipped in the same way as timeouts `#[cfg_attr(test, mutants::skip)]` + +```rust +// Define the Car struct with appropriate field types +#[derive(Debug, Clone)] +struct Car { + color: String, + model: String, + year: i64, +} + +// Manually implement the Default trait for Car +impl Default for Car { + fn default() -> Self { + Self { + color: "Black".to_string(), + model: "Generic Model".to_string(), + year: 2020, // Specific default year + } + } +} + +impl Car { + // Constructor to create a new Car instance with specific attributes + fn new(color: &str, model: &str, year: i64) -> Self { + Self { + color: color.to_string(), + model: model.to_string(), + year, + } + } +} + +// Example usage of Car +fn main() { + // Create a default Car using the Default trait + let default_car = Car::default(); + println!("Default car: {:?}", default_car); + + // Create a custom Car using the new constructor + let custom_car = Car::new("Red", "Ferrari", 2022); + println!("Custom car: {:?}", custom_car); +} + +``` + +### Contribution Prerequisites + +... 🚧 Work in progress 🚧 ...