From e74b9215d98a2de54e3f0666b6617d12c4222835 Mon Sep 17 00:00:00 2001 From: Anurag Thakur Date: Mon, 30 Dec 2024 16:19:37 +0530 Subject: [PATCH] Address review comments --- .github/workflows/cypress-tests-runner.yml | 12 +- docs/CONTRIBUTING.md | 23 ++- justfile | 2 - scripts/execute_cypress.sh | 11 +- scripts/execute_cypress_v2.sh | 188 --------------------- 5 files changed, 30 insertions(+), 206 deletions(-) delete mode 100755 scripts/execute_cypress_v2.sh diff --git a/.github/workflows/cypress-tests-runner.yml b/.github/workflows/cypress-tests-runner.yml index a4843bc2a01a..d804763195a9 100644 --- a/.github/workflows/cypress-tests-runner.yml +++ b/.github/workflows/cypress-tests-runner.yml @@ -20,7 +20,6 @@ env: RUN_TESTS: ${{ ((github.event_name == 'pull_request') && (github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name)) || (github.event_name == 'merge_group')}} DEBUG: cypress:cli RUST_MIN_STACK: 10485760 - CODECOV_FILE: "lcov.info" jobs: formatter: @@ -259,6 +258,8 @@ jobs: runner_v2: name: Run Cypress tests on v2 and generate coverage report runs-on: ubuntu-latest + env: + CODECOV_FILE: "lcov.info" services: redis: @@ -336,7 +337,7 @@ jobs: uses: dtolnay/rust-toolchain@master with: toolchain: stable 2 weeks ago - components: clippy llvm-tools-preview + components: llvm-tools-preview - name: Install sccache if: ${{ env.RUN_TESTS == 'true' }} @@ -355,9 +356,10 @@ jobs: - name: Install grcov if: ${{ env.RUN_TESTS == 'true' }} - uses: baptiste0928/cargo-install@v3.1.1 + uses: taiki-e/install-action@v2.41.10 with: - crate: grcov + tool: grcov + checksum: true - name: Install Just if: ${{ env.RUN_TESTS == 'true' }} @@ -428,7 +430,7 @@ jobs: shell: bash -leuo pipefail {0} continue-on-error: true run: | - scripts/execute_cypress_v2.sh + scripts/execute_cypress_v2.sh "" "" "cypress-tests-v2" - name: Stop running server if: ${{ env.RUN_TESTS == 'true' }} && always() diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 61db1a028a29..9b7b6790e3c8 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -225,42 +225,51 @@ cargo +nightly fmt ### Code Coverage -We value well tested code. You should try to add tests for the code you add. +We appreciate well-tested code, so feel free to add tests when you can. -For generating code coverage, follow the following steps: +To generate code coverage using the cypress tests, follow these steps: - Make sure `grcov` and `llvm-tools-preview` are installed + ```shell + rustup install llvm-tools-preview + cargo install grcov + ``` + - Build the project with the `-Cinstrument-coverage` flag: ```shell RUSTFLAGS="-Cinstrument-coverage" cargo build --bin=router --package=router ``` - Several `.profraw` files will be generated. + Several `.profraw` files will be generated. (Due to the execution of build scripts) + +- Execute the binary: -- Run the project using: ```shell LLVM_PROFILE_FILE="coverage.profraw" target/debug/router ``` -- Open a separate terminal tab and run the cypress tests, following the README +- Open a separate terminal tab and run the cypress tests, following the [README] - After the tests have finished running, stop the `router` process using `Ctrl+C` - The generated `coverage.profraw` file will contain the code coverage data for `router` -- Generate an html report from the data using: +- Generate an html report from the data: + ```shell grcov . --source-dir . --output-type html --binary-path ./target/debug ``` - A folder named `html` will be generated, containing the report. You can view it using: + ```shell cd html && python3 -m http.server 8000 ``` - You can delete the generated `.profraw` files using: + ```shell rm **/*.profraw ``` @@ -269,6 +278,8 @@ Note: - It is necessary to stop the `router` process to generate the coverage file - Branch coverage generation requires nightly and currently `grcov` crashes while trying to include branch coverage. (Checked using `--log-level` parameter in `grcov`) +[README]: /cypress-tests-v2/README.md + ### Commits It is a recommended best practice to keep your changes as logically grouped as diff --git a/justfile b/justfile index eeb465152c33..5a0d13396ba6 100644 --- a/justfile +++ b/justfile @@ -79,8 +79,6 @@ build_v2 *FLAGS: cargo build --package router --bin router --no-default-features --features "${FEATURES}" {{ FLAGS }} set +x -alias bb := build_v2 - run_v2: #! /usr/bin/env bash diff --git a/scripts/execute_cypress.sh b/scripts/execute_cypress.sh index 1f1219ee717c..8d93519b468c 100755 --- a/scripts/execute_cypress.sh +++ b/scripts/execute_cypress.sh @@ -158,12 +158,13 @@ function cleanup() { function main() { local command="${1:-}" local jobs="${2:-5}" + local test_dir="${3:-cypress-tests}" - # Ensure script runs from 'cypress-tests' directory - if [[ "$(basename "$PWD")" != "cypress-tests" ]]; then - print_color "yellow" "Changing directory to 'cypress-tests'..." - cd cypress-tests || { - print_color "red" "ERROR: Directory 'cypress-tests' not found!" + # Ensure script runs from the specified test directory (default: cypress-tests) + if [[ "$(basename "$PWD")" != "$(basename "$test_dir")" ]]; then + print_color "yellow" "Changing directory to '${test_dir}'..." + cd "${test_dir}" || { + print_color "red" "ERROR: Directory '${test_dir}' not found!" exit 1 } fi diff --git a/scripts/execute_cypress_v2.sh b/scripts/execute_cypress_v2.sh deleted file mode 100755 index bcd6664e59f0..000000000000 --- a/scripts/execute_cypress_v2.sh +++ /dev/null @@ -1,188 +0,0 @@ -#! /usr/bin/env bash - -set -euo pipefail - -# Initialize tmp_file globally -tmp_file="" - -# Define arrays for services, etc. -# Read service arrays from environment variables -read -r -a payments <<< "${PAYMENTS_CONNECTORS[@]:-}" -read -r -a payouts <<< "${PAYOUTS_CONNECTORS[@]:-}" -read -r -a payment_method_list <<< "${PAYMENT_METHOD_LIST[@]:-}" -read -r -a routing <<< "${ROUTING[@]:-}" - -# Define arrays -connector_map=() -failed_connectors=() - -# Define an associative array to map environment variables to service names -declare -A services=( - ["PAYMENTS_CONNECTORS"]="payments" - ["PAYOUTS_CONNECTORS"]="payouts" - ["PAYMENT_METHOD_LIST"]="payment_method_list" - ["ROUTING"]="routing" -) - -# Function to print messages in color -function print_color() { - # Input params - local color="$1" - local message="$2" - - # Define colors - local reset='\033[0m' - local red='\033[0;31m' - local green='\033[0;32m' - local yellow='\033[0;33m' - - # Use indirect reference to get the color value - echo -e "${!color}${message}${reset}" -} -export -f print_color - -# Function to check if a command exists -function command_exists() { - command -v "$1" > /dev/null 2>&1 -} - -# Function to read service arrays from environment variables -function read_service_arrays() { - # Loop through the associative array and check if each service is exported - for var in "${!services[@]}"; do - if [[ -n "${!var+x}" ]]; then - connector_map+=("${services[$var]}") - else - print_color "yellow" "Environment variable ${var} is not set. Skipping..." - fi - done -} - -# Function to execute Cypress tests -function execute_test() { - if [[ $# -lt 3 ]]; then - print_color "red" "ERROR: Insufficient arguments provided to execute_test." - exit 1 - fi - - local connector="$1" - local service="$2" - local tmp_file="$3" - - print_color "yellow" "Executing tests for ${service} with connector ${connector}..." - - export REPORT_NAME="${service}_${connector}_report" - - if ! CYPRESS_CONNECTOR="$connector" npm run "cypress:$service"; then - echo "${service}-${connector}" >> "${tmp_file}" - fi -} -export -f execute_test - -# Function to run tests -function run_tests() { - local jobs="${1:-1}" - tmp_file=$(mktemp) - - # Ensure temporary file is removed on script exit - trap 'cleanup' EXIT - - for service in "${connector_map[@]}"; do - declare -n connectors="$service" - - if [[ ${#connectors[@]} -eq 0 ]]; then - # Service-level test (e.g., payment-method-list or routing) - [[ $service == "payment_method_list" ]] && service="payment-method-list" - - echo "Running ${service} tests without connectors..." - export REPORT_NAME="${service}_report" - - if ! npm run "cypress:${service}"; then - echo "${service}" >> "${tmp_file}" - fi - else - # Connector-specific tests (e.g., payments or payouts) - print_color "yellow" "Running tests for service: '${service}' with connectors: [${connectors[*]}] in batches of ${jobs}..." - - # Execute tests in parallel - printf '%s\n' "${connectors[@]}" | parallel --jobs "${jobs}" execute_test {} "${service}" "${tmp_file}" - fi - done - - # Collect failed connectors - if [[ -s "${tmp_file}" ]]; then - failed_connectors=($(< "${tmp_file}")) - print_color "red" "One or more connectors failed to run:" - printf '%s\n' "${failed_connectors[@]}" - exit 1 - else - print_color "green" "Cypress tests execution successful!" - fi -} - -# Function to check and install dependencies -function check_dependencies() { - # parallel and npm are mandatory dependencies. exit the script if not found. - local dependencies=("parallel" "npm") - - for cmd in "${dependencies[@]}"; do - if ! command_exists "$cmd"; then - print_color "red" "ERROR: ${cmd^} is not installed!" - exit 1 - else - print_color "green" "${cmd^} is installed already!" - - if [[ ${cmd} == "npm" ]]; then - npm ci || { - print_color "red" "Command \`npm ci\` failed!" - exit 1 - } - fi - fi - done -} - -# Cleanup function to handle exit -function cleanup() { - print_color "yellow" "Cleaning up..." - if [[ -d "cypress-tests" ]]; then - cd - - fi - - if [[ -n "${tmp_file}" && -f "${tmp_file}" ]]; then - rm -f "${tmp_file}" - fi -} - -# Main function -function main() { - local command="${1:-}" - local jobs="${2:-5}" - - # Ensure script runs from 'cypress-tests' directory - if [[ "$(basename "$PWD")" != "cypress-tests-v2" ]]; then - print_color "yellow" "Changing directory to 'cypress-tests-v2'..." - cd cypress-tests-v2 || { - print_color "red" "ERROR: Directory 'cypress-tests-v2' not found!" - exit 1 - } - fi - - check_dependencies - read_service_arrays - - case "$command" in - --parallel | -p) - print_color "yellow" "WARNING: Running Cypress tests in parallel is more resource-intensive!" - # At present, parallel execution is restricted to not run out of memory - # But can be scaled up by passing the value as an argument - run_tests "$jobs" - ;; - *) - run_tests 1 - ;; - esac -} - -# Execute the main function with passed arguments -main "$@"