From a0c684573bc31b94d410dcfef5ee8b8db23f9438 Mon Sep 17 00:00:00 2001 From: Lukas Dullinger <129603980+itislu@users.noreply.github.com> Date: Mon, 29 Jul 2024 08:40:18 +0200 Subject: [PATCH 1/5] ci: Use `inputs` for 'setup' action to enforce correct use Making the inputs optional and checking if they are set before using them allows to set up less when not needed. An example is the compilation test. --- .github/actions/setup/action.yaml | 21 +++++++++++++++++---- .github/workflows/regression_test.yaml | 3 +++ .github/workflows/test.yaml | 3 +++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/.github/actions/setup/action.yaml b/.github/actions/setup/action.yaml index 17e8ebec..ca1f278c 100644 --- a/.github/actions/setup/action.yaml +++ b/.github/actions/setup/action.yaml @@ -1,6 +1,15 @@ # .github/actions/setup/action.yaml name: Set Up Test Environment Action description: Setup action + +inputs: + tester_dir: + description: Directory where the tester will be installed to + required: false + scripts_dir: + description: Directory where the scripts for analyzing the test results will be copied to + required: false + runs: using: composite steps: @@ -17,9 +26,13 @@ runs: shell: bash - run: env --list-signal-handling bash shell: bash - - run: | - git clone https://github.com/LeaYeh/42_minishell_tester.git ${{ env.TESTER_DIR }} - chmod +x ${{ env.TESTER_DIR }}/tester.sh + - if: inputs.tester_dir + run: | + git clone https://github.com/LeaYeh/42_minishell_tester.git ${{ inputs.tester_dir }} + chmod +x ${{ inputs.tester_dir }}/tester.sh + shell: bash + - if: inputs.scripts_dir + run: | find .github/scripts -type f -name "*.sh" -exec chmod +x {} \; - cp -r .github/scripts ${{ env.SCRIPTS_DIR }} + cp -r .github/scripts ${{ inputs.scripts_dir }} shell: bash diff --git a/.github/workflows/regression_test.yaml b/.github/workflows/regression_test.yaml index 1a54b63b..f35d8ff9 100644 --- a/.github/workflows/regression_test.yaml +++ b/.github/workflows/regression_test.yaml @@ -41,6 +41,9 @@ jobs: - name: Set up test environment uses: ./.github/actions/setup + with: + tester_dir: ${{ env.TESTER_DIR }} + scripts_dir: ${{ env.SCRIPTS_DIR }} - name: 🌱 Test source branch of pull request run: | diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index bb171386..31fe6b24 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -69,6 +69,9 @@ jobs: uses: actions/checkout@v4 - name: Set up test environment uses: ./.github/actions/setup + with: + tester_dir: ${{ env.TESTER_DIR }} + scripts_dir: ${{ env.SCRIPTS_DIR }} - name: Delete all but one test script run: | for file in ${{ env.TESTER_DIR }}/cmds/**/*.sh; do From 059348e6444d353057bb40b9f04d8f02746bc363 Mon Sep 17 00:00:00 2001 From: Lukas Dullinger <129603980+itislu@users.noreply.github.com> Date: Mon, 29 Jul 2024 08:52:57 +0200 Subject: [PATCH 2/5] ci: Use `inputs` for 'summary' action to enforce correct use Using env was hard to follow and required comments in the action what was required for that action to work. --- .../actions/summary_test_result/action.yaml | 44 +++++++++++++++---- .../scripts/print_all_failed_test_cases.sh | 6 +-- .github/scripts/print_changed_test_cases.sh | 2 +- .github/scripts/print_leak_test_cases.sh | 6 +-- .github/workflows/regression_test.yaml | 26 ++++++----- .github/workflows/test.yaml | 12 ++--- 6 files changed, 65 insertions(+), 31 deletions(-) diff --git a/.github/actions/summary_test_result/action.yaml b/.github/actions/summary_test_result/action.yaml index e479688f..3773eaf8 100644 --- a/.github/actions/summary_test_result/action.yaml +++ b/.github/actions/summary_test_result/action.yaml @@ -1,16 +1,46 @@ # .github/actions/summary_test_result/action.yaml name: Test Result Summary Action description: Compares failed counts between source and target branches + +inputs: + source_failed_count: + description: Number of failed test cases on source branch + required: true + target_failed_count: + description: Number of failed test cases on target branch + required: true + source_log_file: + description: Path to the log file for the source branch + required: true + target_log_file: + description: Path to the log file for the target branch + required: true + tester_dir: + description: Directory containing the tester + required: true + tester_output_dir: + description: Directory containing the tester output files + required: true + scripts_dir: + description: Directory containing the scripts for analyzing the test results + required: true + +env: + SOURCE_LOG_FILE: ${{ inputs.source_log_file }} + TARGET_LOG_FILE: ${{ inputs.target_log_file }} + TESTER_DIR: ${{ inputs.tester_dir }} + TESTER_OUTPUT_DIR: ${{ inputs.tester_output_dir }} + SCRIPTS_DIR: ${{ inputs.scripts_dir }} + runs: using: composite steps: - # Requires SOURCE_FAILED_COUNT and TARGET_FAILED_COUNT environment variables - name: 🆚 Compare failed count id: comparison run: | - echo -e "\e[93m🌱 SOURCE_FAILED_COUNT: $SOURCE_FAILED_COUNT\e[0m" - echo -e "\e[94m🎯 TARGET_FAILED_COUNT: $TARGET_FAILED_COUNT\e[0m" - if [ $SOURCE_FAILED_COUNT -gt $TARGET_FAILED_COUNT ]; then + echo -e "\e[93m🌱 SOURCE_FAILED_COUNT: ${{ inputs.source_failed_count }}\e[0m" + echo -e "\e[94m🎯 TARGET_FAILED_COUNT: ${{ inputs.target_failed_count }}\e[0m" + if [ ${{ inputs.source_failed_count }} -gt ${{ inputs.target_failed_count }} ]; then echo -e "\e[1;31mSOURCE_FAILED_COUNT is greater than TARGET_FAILED_COUNT\e[0m" exit 1 else @@ -19,13 +49,11 @@ runs: fi continue-on-error: true shell: bash - # Requires TESTER_OUTPUT_DIR environment variable - name: 📈 Show the regressions between source and target branch if: steps.comparison.outcome == 'failure' - run: ${{ env.SCRIPTS_DIR }}/print_changed_test_cases.sh + run: ${{ inputs.scripts_dir }}/print_changed_test_cases.sh shell: bash - # Requires TESTER_OUTPUT_DIR environment variable - name: 📉 Show the improvements between source and target branch if: steps.comparison.outcome == 'success' - run: ${{ env.SCRIPTS_DIR }}/print_changed_test_cases.sh || true + run: ${{ inputs.scripts_dir }}/print_changed_test_cases.sh || true shell: bash diff --git a/.github/scripts/print_all_failed_test_cases.sh b/.github/scripts/print_all_failed_test_cases.sh index fff5a169..43205729 100755 --- a/.github/scripts/print_all_failed_test_cases.sh +++ b/.github/scripts/print_all_failed_test_cases.sh @@ -1,9 +1,9 @@ #!/bin/bash -test_output=$(cat "$RESULT_FILE") +test_log=$(cat "$LOG_FILE") result=0 -# Extract line numbers and file paths from test output +# Extract line numbers and file paths from test log while IFS= read -r line; do if [[ $line == *"❌"* ]]; then echo "$line" @@ -19,4 +19,4 @@ while IFS= read -r line; do if [[ $result -ne 0 ]]; then echo "$line" fi -done <<< "$test_output" +done <<< "$test_log" diff --git a/.github/scripts/print_changed_test_cases.sh b/.github/scripts/print_changed_test_cases.sh index 97a10e35..b8d25104 100755 --- a/.github/scripts/print_changed_test_cases.sh +++ b/.github/scripts/print_changed_test_cases.sh @@ -1,6 +1,6 @@ #!/bin/bash -diff_output=$(diff "$HOME/target_test_result.txt" "$HOME/source_test_result.txt" || true) +diff_output=$(diff "$TARGET_LOG_FILE" "$SOURCE_LOG_FILE" || true) # Extract line numbers and file paths from diff output while IFS= read -r line; do diff --git a/.github/scripts/print_leak_test_cases.sh b/.github/scripts/print_leak_test_cases.sh index f4601754..8cc346a6 100755 --- a/.github/scripts/print_leak_test_cases.sh +++ b/.github/scripts/print_leak_test_cases.sh @@ -1,10 +1,10 @@ #!/bin/bash -test_output=$(cat "$RESULT_FILE") +test_log=$(cat "$LOG_FILE") leaks=0 result=0 -# Extract line numbers and file paths from test output +# Extract line numbers and file paths from test log while IFS= read -r line; do stripped_line=$(echo "$line" | sed 's/\x1b\[[0-9;]*m//g') if [[ $stripped_line == *"LEAKS: ❌"* ]]; then @@ -22,7 +22,7 @@ while IFS= read -r line; do if [[ $result -ne 0 ]]; then echo "$line" fi -done <<< "$test_output" +done <<< "$test_log" if [[ $leaks -ne 0 ]] ; then exit 1 diff --git a/.github/workflows/regression_test.yaml b/.github/workflows/regression_test.yaml index f35d8ff9..a62bf7b1 100644 --- a/.github/workflows/regression_test.yaml +++ b/.github/workflows/regression_test.yaml @@ -23,9 +23,12 @@ on: env: HOME: /home/runner TESTER_DIR: /home/runner/42_minishell_tester + TESTER_OUTPUT_DIR: /home/runner/tester_output SCRIPTS_DIR: /home/runner/scripts SOURCE_FAILED_COUNT: 0 TARGET_FAILED_COUNT: 0 + SOURCE_LOG_FILE: /home/runner/source_test.log + TARGET_LOG_FILE: /home/runner/target_test.log jobs: regression_test: @@ -48,18 +51,17 @@ jobs: - name: 🌱 Test source branch of pull request run: | make re CC=clang-12 - ${{ env.TESTER_DIR }}/tester.sh --no-update ${{ inputs.test_mode }} > ${{ env.HOME }}/source_test_result.txt + ${{ env.TESTER_DIR }}/tester.sh --no-update ${{ inputs.test_mode }} > ${{ env.SOURCE_LOG_FILE }} env: GH_BRANCH: SOURCE_FAILED_COUNT - - name: Save tester output to home directory - run: mv -f tester_output* ${{ env.HOME }}/tester_output || true + - name: Save tester output + run: mv -f tester_output* ${{ env.TESTER_OUTPUT_DIR }} || true - name: 📝 Print all test cases that failed on source branch run: ${{ env.SCRIPTS_DIR }}/print_all_failed_test_cases.sh env: - RESULT_FILE: ${{ env.HOME }}/source_test_result.txt - TESTER_OUTPUT_DIR: ${{ env.HOME }}/tester_output + LOG_FILE: ${{ env.SOURCE_LOG_FILE }} - name: Checkout target branch of pull request uses: actions/checkout@v4 @@ -69,7 +71,7 @@ jobs: - name: 🎯 Test target branch of pull request run: | make re CC=clang-12 - ${{ env.TESTER_DIR }}/tester.sh --no-update ${{ inputs.test_mode }} > ${{ env.HOME }}/target_test_result.txt + ${{ env.TESTER_DIR }}/tester.sh --no-update ${{ inputs.test_mode }} > ${{ env.TARGET_LOG_FILE }} env: GH_BRANCH: TARGET_FAILED_COUNT @@ -78,7 +80,11 @@ jobs: - name: 📜 Summarize regression test result uses: ./.github/actions/summary_test_result - env: - SOURCE_FAILED_COUNT: ${{ env.SOURCE_FAILED_COUNT }} - TARGET_FAILED_COUNT: ${{ env.TARGET_FAILED_COUNT }} - TESTER_OUTPUT_DIR: ${{ env.HOME }}/tester_output + with: + source_failed_count: ${{ env.SOURCE_FAILED_COUNT }} + target_failed_count: ${{ env.TARGET_FAILED_COUNT }} + source_log_file: ${{ env.SOURCE_LOG_FILE }} + target_log_file: ${{ env.TARGET_LOG_FILE }} + tester_dir: ${{ env.TESTER_DIR }} + tester_output_dir: ${{ env.TESTER_OUTPUT_DIR }} + scripts_dir: ${{ env.SCRIPTS_DIR }} diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 31fe6b24..52f5bc91 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -64,6 +64,9 @@ jobs: fail-fast: false matrix: test_script: ${{ fromJson(needs.prepare_test_matrix.outputs.test_matrix) }} + env: + TESTER_OUTPUT_DIR: /home/runner/tester_output + LOG_FILE: /home/runner/leak_test.log steps: - name: Checkout source branch of pull request uses: actions/checkout@v4 @@ -82,17 +85,14 @@ jobs: - name: 🔍 Check memory leaks run: | make re CC=clang-12 - ${{ env.TESTER_DIR }}/tester.sh --no-update va > ${{ env.HOME }}/leak_result.txt + ${{ env.TESTER_DIR }}/tester.sh --no-update va > ${{ env.LOG_FILE }} env: GH_BRANCH: IGNORE continue-on-error: true - - name: Save tester output to home directory - run: mv -f tester_output* ${{ env.HOME }}/tester_output || true + - name: Save tester output + run: mv -f tester_output* ${{ env.TESTER_OUTPUT_DIR }} || true - name: 📝 Print all test cases that leaked on source branch run: ${{ env.SCRIPTS_DIR }}/print_leak_test_cases.sh - env: - RESULT_FILE: ${{ env.HOME }}/leak_result.txt - TESTER_OUTPUT_DIR: ${{ env.HOME }}/tester_output combine_memory_leak_test_results: name: Combine Memory Leak Test Results From bd2c926f05dd5760347de077ccab4de9b33a6bcf Mon Sep 17 00:00:00 2001 From: Lukas Dullinger <129603980+itislu@users.noreply.github.com> Date: Mon, 29 Jul 2024 09:15:44 +0200 Subject: [PATCH 3/5] ci: Fix scope of env for setup action --- .../actions/summary_test_result/action.yaml | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/.github/actions/summary_test_result/action.yaml b/.github/actions/summary_test_result/action.yaml index 3773eaf8..423cb1b1 100644 --- a/.github/actions/summary_test_result/action.yaml +++ b/.github/actions/summary_test_result/action.yaml @@ -25,18 +25,14 @@ inputs: description: Directory containing the scripts for analyzing the test results required: true -env: - SOURCE_LOG_FILE: ${{ inputs.source_log_file }} - TARGET_LOG_FILE: ${{ inputs.target_log_file }} - TESTER_DIR: ${{ inputs.tester_dir }} - TESTER_OUTPUT_DIR: ${{ inputs.tester_output_dir }} - SCRIPTS_DIR: ${{ inputs.scripts_dir }} - runs: using: composite steps: - name: 🆚 Compare failed count id: comparison + env: + SOURCE_FAILED_COUNT: ${{ inputs.source_failed_count }} + TARGET_FAILED_COUNT: ${{ inputs.target_failed_count }} run: | echo -e "\e[93m🌱 SOURCE_FAILED_COUNT: ${{ inputs.source_failed_count }}\e[0m" echo -e "\e[94m🎯 TARGET_FAILED_COUNT: ${{ inputs.target_failed_count }}\e[0m" @@ -52,8 +48,16 @@ runs: - name: 📈 Show the regressions between source and target branch if: steps.comparison.outcome == 'failure' run: ${{ inputs.scripts_dir }}/print_changed_test_cases.sh + env: + TESTER_DIR: ${{ inputs.tester_dir }} + TESTER_OUTPUT_DIR: ${{ inputs.tester_output_dir }} + SCRIPTS_DIR: ${{ inputs.scripts_dir }} shell: bash - name: 📉 Show the improvements between source and target branch if: steps.comparison.outcome == 'success' run: ${{ inputs.scripts_dir }}/print_changed_test_cases.sh || true + env: + TESTER_DIR: ${{ inputs.tester_dir }} + TESTER_OUTPUT_DIR: ${{ inputs.tester_output_dir }} + SCRIPTS_DIR: ${{ inputs.scripts_dir }} shell: bash From cddfd1422e4a0761968eb07b1c90cfb5b136547d Mon Sep 17 00:00:00 2001 From: Lukas Dullinger <129603980+itislu@users.noreply.github.com> Date: Mon, 29 Jul 2024 10:27:42 +0200 Subject: [PATCH 4/5] ci: Use uppercase for global env variables in delivery.yaml and lower case for local variables. --- .github/workflows/delivery.yaml | 44 ++++++++++++++++----------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/.github/workflows/delivery.yaml b/.github/workflows/delivery.yaml index 768a1383..bbc2375b 100644 --- a/.github/workflows/delivery.yaml +++ b/.github/workflows/delivery.yaml @@ -10,8 +10,8 @@ jobs: update_evaluation_branch: runs-on: ubuntu-latest env: - proceed: - main_tag: + PROCEED: + MAIN_TAG: steps: - name: Checkout code @@ -26,61 +26,61 @@ jobs: - name: Check tag version run: | - MAIN_TAG=$(git tag --sort=-creatordate --merged origin/main 2>/dev/null | head -n 1) || true - EVAL_TAG=$(git tag --sort=-creatordate --no-merged origin/main 2>/dev/null | head -n 1) || true - echo "MAIN_TAG: $MAIN_TAG" - echo "EVAL_TAG: $EVAL_TAG" - echo main_tag=$MAIN_TAG >> "$GITHUB_ENV" - if [[ "$MAIN_TAG" > "$EVAL_TAG" ]]; then - echo "proceed=true" >> "$GITHUB_ENV" + main_tag=$(git tag --sort=-creatordate --merged origin/main 2>/dev/null | head -n 1) || true + eval_tag=$(git tag --sort=-creatordate --no-merged origin/main 2>/dev/null | head -n 1) || true + echo "MAIN_TAG: $main_tag" + echo "EVAL_TAG: $eval_tag" + echo MAIN_TAG=$main_tag >> "$GITHUB_ENV" + if [[ "$main_tag" > "$eval_tag" ]]; then + echo "PROCEED=true" >> "$GITHUB_ENV" else - echo "proceed=false" >> "$GITHUB_ENV" + echo "PROCEED=false" >> "$GITHUB_ENV" fi - name: Save .github directory - if: env.proceed == 'true' + if: env.PROCEED == 'true' run: | cp -rf .github ~/.github - name: Setup evaluation branch - if: env.proceed == 'true' + if: env.PROCEED == 'true' run: | git checkout -b evaluation origin/evaluation || git checkout -b evaluation git merge origin/main || (git checkout --theirs . && git add -A) - name: Restore .github directory - if: env.proceed == 'true' + if: env.PROCEED == 'true' run: | rm -rf .github mv ~/.github .github - name: Partial cleanup of forbidden files - if: env.proceed == 'true' + if: env.PROCEED == 'true' uses: ./.github/actions/cleanup_partial - name: Commit partial cleanup to evaluation branch - if: env.proceed == 'true' + if: env.PROCEED == 'true' run: | git add -A git commit -m "[GH-BOT] Remove forbidden files and empty folders" - name: 📏 Check norminette - if: env.proceed == 'true' + if: env.PROCEED == 'true' uses: ./.github/actions/norminette - name: Full cleanup of forbidden files - if: env.proceed == 'true' + if: env.PROCEED == 'true' uses: ./.github/actions/cleanup_full - name: Amend full cleanup to evaluation branch - if: env.proceed == 'true' + if: env.PROCEED == 'true' run: | git add -A git commit --amend --no-edit - name: 🏷️ Set version tag on evaluation branch - if: env.proceed == 'true' + if: env.PROCEED == 'true' run: | - EVAL_TAG="${{ env.main_tag }}-eval" - git tag "$EVAL_TAG" - git push origin evaluation "$EVAL_TAG" + eval_tag="${{ env.MAIN_TAG }}-eval" + git tag "$eval_tag" + git push origin evaluation "$eval_tag" From d38567ed0e2d5acf8fedafdab74dae304fca5474 Mon Sep 17 00:00:00 2001 From: Lukas Dullinger <129603980+itislu@users.noreply.github.com> Date: Mon, 29 Jul 2024 11:02:31 +0200 Subject: [PATCH 5/5] ci: Split up delivery workflow into 2 jobs - Using jobs for controling the flow is better GitHub Actions practice than constant if checks. - Make the names of the jobs and steps more clear. --- .github/workflows/delivery.yaml | 53 +++++++++++++++++---------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/.github/workflows/delivery.yaml b/.github/workflows/delivery.yaml index bbc2375b..d3083bae 100644 --- a/.github/workflows/delivery.yaml +++ b/.github/workflows/delivery.yaml @@ -7,80 +7,83 @@ on: - 'v*' jobs: - update_evaluation_branch: + check_tag_version: + name: Check Tag Version runs-on: ubuntu-latest - env: - PROCEED: - MAIN_TAG: - + outputs: + main_tag: ${{ steps.check_new_version.outputs.main_tag }} + is_new_version: ${{ steps.check_new_version.outputs.is_new_version }} steps: - - name: Checkout code + - name: Checkout tag uses: actions/checkout@v4 with: fetch-depth: 0 - - name: Setup Git - run: | - git config --local user.email "lyeh@student.42vienna.com" - git config --local user.name "Lea Yeh" - - - name: Check tag version + - name: Check if new version + id: check_new_version run: | main_tag=$(git tag --sort=-creatordate --merged origin/main 2>/dev/null | head -n 1) || true eval_tag=$(git tag --sort=-creatordate --no-merged origin/main 2>/dev/null | head -n 1) || true echo "MAIN_TAG: $main_tag" echo "EVAL_TAG: $eval_tag" - echo MAIN_TAG=$main_tag >> "$GITHUB_ENV" + echo "main_tag=$main_tag" >> $GITHUB_OUTPUT if [[ "$main_tag" > "$eval_tag" ]]; then - echo "PROCEED=true" >> "$GITHUB_ENV" + echo "is_new_version=true" >> $GITHUB_OUTPUT else - echo "PROCEED=false" >> "$GITHUB_ENV" + echo "is_new_version=false" >> $GITHUB_OUTPUT fi + update_evaluation_branch: + name: Update Evaluation Branch + needs: check_tag_version + if: needs.check_tag_version.outputs.is_new_version == 'true' + runs-on: ubuntu-latest + steps: + - name: Checkout tag + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Setup Git + run: | + git config --local user.email "lyeh@student.42vienna.com" + git config --local user.name "Lea Yeh" + - name: Save .github directory - if: env.PROCEED == 'true' run: | cp -rf .github ~/.github - name: Setup evaluation branch - if: env.PROCEED == 'true' run: | git checkout -b evaluation origin/evaluation || git checkout -b evaluation git merge origin/main || (git checkout --theirs . && git add -A) - name: Restore .github directory - if: env.PROCEED == 'true' run: | rm -rf .github mv ~/.github .github - name: Partial cleanup of forbidden files - if: env.PROCEED == 'true' uses: ./.github/actions/cleanup_partial - name: Commit partial cleanup to evaluation branch - if: env.PROCEED == 'true' run: | git add -A git commit -m "[GH-BOT] Remove forbidden files and empty folders" - name: 📏 Check norminette - if: env.PROCEED == 'true' uses: ./.github/actions/norminette - name: Full cleanup of forbidden files - if: env.PROCEED == 'true' uses: ./.github/actions/cleanup_full - name: Amend full cleanup to evaluation branch - if: env.PROCEED == 'true' run: | git add -A git commit --amend --no-edit - name: 🏷️ Set version tag on evaluation branch - if: env.PROCEED == 'true' run: | - eval_tag="${{ env.MAIN_TAG }}-eval" + eval_tag="${{ needs.check_tag_version.outputs.main_tag }}-eval" git tag "$eval_tag" git push origin evaluation "$eval_tag"