From 0c6e9a5778e1f970c7f2c88dec755b988e74ea20 Mon Sep 17 00:00:00 2001 From: Marco Ferrari Date: Mon, 12 Aug 2024 18:01:40 +0200 Subject: [PATCH] chore: remove test leftovers before running fix test (#5995) - Remove test leftovers before initializing the workspace against which fix mode tests run. This prevents ownership issues. - Pass container image build metadata as outputs of the container image build job so we can include the validate-container-image-labels target as other test target when we build the test matrix. - Manually include the 'test' target when building the test suite matrix so we run it as part of the test suite to ensure that tests don't pollute each other's working directory. Fix #5994 --- .github/workflows/ci.yml | 110 +++++++++++++++++++++------------ test/run-super-linter-tests.sh | 35 +++-------- test/testUtils.sh | 42 +++++++++++++ 3 files changed, 120 insertions(+), 67 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 06ea67df..cd0c148c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,11 +10,70 @@ on: permissions: {} jobs: + # Set build metadata before running any other job so we can reuse them for + # both the standandard and the slim images, and for verification as well. + # We can't run this as part of the build-container-image job because it + # runs multiple times due to its matrix configuration, leading to race + # conditions when verifying container image labels. + set-build-metadata: + name: Set build metadata + runs-on: ubuntu-latest + permissions: + contents: read + outputs: + CONTAINER_IMAGE_BUILD_DATE: ${{ steps.set-container-image-build-metadata.outputs.CONTAINER_IMAGE_BUILD_DATE }} + CONTAINER_IMAGE_BUILD_REVISION: ${{ steps.set-container-image-build-metadata.outputs.CONTAINER_IMAGE_BUILD_REVISION }} + CONTAINER_IMAGE_BUILD_VERSION: ${{ steps.set-container-image-build-metadata.outputs.CONTAINER_IMAGE_BUILD_VERSION }} + steps: + - name: Set build metadata + id: set-container-image-build-metadata + run: | + if [[ ${{ github.event_name }} == 'push' ]] || [[ ${{ github.event_name }} == 'merge_group' ]]; then + BUILD_REVISION=${{ github.sha }} + BUILD_VERSION=${{ github.sha }} + elif [[ ${{ github.event_name }} == 'pull_request' ]]; then + BUILD_REVISION=${{ github.event.pull_request.head.sha }} + BUILD_VERSION=${{ github.event.pull_request.head.sha }} + else + echo "[ERROR] Event not supported when setting build revision and build version" + exit 1 + fi + + if [ -z "${BUILD_REVISION}" ]; then + echo "[ERROR] BUILD_REVISION is empty" + exit 1 + fi + + if [ -z "${BUILD_VERSION}" ]; then + echo "[ERROR] BUILD_VERSION is empty" + exit 1 + fi + + BUILD_DATE=$(date -u +'%Y-%m-%dT%H:%M:%SZ') + + echo "Build date: ${BUILD_DATE}" + echo "Build revision: ${BUILD_REVISION}" + echo "Build version: ${BUILD_VERSION}" + + { + echo "BUILD_DATE=${BUILD_DATE}" + echo "BUILD_REVISION=${BUILD_REVISION}" + echo "BUILD_VERSION=${BUILD_VERSION}" + } >> "${GITHUB_ENV}" + + { + echo "CONTAINER_IMAGE_BUILD_DATE=${BUILD_DATE}" + echo "CONTAINER_IMAGE_BUILD_REVISION=${BUILD_REVISION}" + echo "CONTAINER_IMAGE_BUILD_VERSION=${BUILD_VERSION}" + } >> "${GITHUB_OUTPUT}" + build-container-image: name: Build and Test runs-on: ubuntu-latest permissions: contents: read + needs: + - set-build-metadata concurrency: # Ref: https://docs.github.com/en/actions/learn-github-actions/contexts#github-context # github.head_ref: head_ref or source branch of the pull request @@ -40,35 +99,6 @@ jobs: with: fetch-depth: 0 - - name: Set build metadata - run: | - if [[ ${{ github.event_name }} == 'push' ]] || [[ ${{ github.event_name }} == 'merge_group' ]]; then - BUILD_REVISION=${{ github.sha }} - BUILD_VERSION=${{ github.sha }} - elif [[ ${{ github.event_name }} == 'pull_request' ]]; then - BUILD_REVISION=${{ github.event.pull_request.head.sha }} - BUILD_VERSION=${{ github.event.pull_request.head.sha }} - else - echo "[ERROR] Event not supported when setting build revision and build version" - exit 1 - fi - - if [ -z "${BUILD_REVISION}" ]; then - echo "[ERROR] BUILD_REVISION is empty" - exit 1 - fi - - if [ -z "${BUILD_VERSION}" ]; then - echo "[ERROR] BUILD_VERSION is empty" - exit 1 - fi - - { - echo "BUILD_DATE=$(date -u +'%Y-%m-%dT%H:%M:%SZ')" - echo "BUILD_REVISION=${BUILD_REVISION}" - echo "BUILD_VERSION=${BUILD_VERSION}" - } >> "${GITHUB_ENV}" - - name: Free Disk space shell: bash run: | @@ -86,9 +116,9 @@ jobs: context: . file: ./Dockerfile build-args: | - BUILD_DATE=${{ env.BUILD_DATE }} - BUILD_REVISION=${{ env.BUILD_REVISION }} - BUILD_VERSION=${{ env.BUILD_VERSION }} + BUILD_DATE=${{ needs.set-build-metadata.outputs.CONTAINER_IMAGE_BUILD_DATE }} + BUILD_REVISION=${{ needs.set-build-metadata.outputs.CONTAINER_IMAGE_BUILD_REVISION }} + BUILD_VERSION=${{ needs.set-build-metadata.outputs.CONTAINER_IMAGE_BUILD_VERSION }} cache-from: type=registry,ref=${{ env.CONTAINER_IMAGE_ID }}-buildcache outputs: type=docker,dest=/tmp/${{ env.CONTAINER_IMAGE_OUTPUT_IMAGE_NAME }}.tar push: false @@ -109,11 +139,6 @@ jobs: run: | make info - # Validate the container image labels here so we don't have to pass the expected - # label values to other build jobs - - name: Validate container image labels - run: make validate-container-image-labels - - name: Upload ${{ env.CONTAINER_IMAGE_OUTPUT_IMAGE_NAME }} container image uses: actions/upload-artifact@v4.3.4 with: @@ -214,6 +239,7 @@ jobs: permissions: contents: read needs: + - set-build-metadata - build-container-image - build-test-suite-matrix strategy: @@ -226,13 +252,17 @@ jobs: - container-image-id: super-linter-slim-latest prefix: slim- target: slim - exclude: - # Don't validate container image labels because we do that when building the image - - test-case: validate-container-image-labels + include: + # Also run the test target to check if running the whole test suite + # in the same execution environment prevents it from succeeding + - test-case: test env: CONTAINER_IMAGE_ID: "ghcr.io/super-linter/super-linter:${{ matrix.images.prefix }}latest" CONTAINER_IMAGE_TARGET: "${{ matrix.images.target }}" CONTAINER_IMAGE_OUTPUT_IMAGE_NAME: "super-linter-${{ matrix.images.prefix }}latest" + BUILD_DATE: ${{ needs.set-build-metadata.outputs.CONTAINER_IMAGE_BUILD_DATE }} + BUILD_REVISION: ${{ needs.set-build-metadata.outputs.CONTAINER_IMAGE_BUILD_REVISION }} + BUILD_VERSION: ${{ needs.set-build-metadata.outputs.CONTAINER_IMAGE_BUILD_VERSION }} steps: - name: Checkout Code uses: actions/checkout@v4 diff --git a/test/run-super-linter-tests.sh b/test/run-super-linter-tests.sh index 1f9a9da1..90b01d66 100755 --- a/test/run-super-linter-tests.sh +++ b/test/run-super-linter-tests.sh @@ -16,8 +16,6 @@ DEFAULT_BRANCH="main" COMMAND_TO_RUN=(docker run -t -e DEFAULT_BRANCH="${DEFAULT_BRANCH}" -e ENABLE_GITHUB_ACTIONS_GROUP_TITLE=true) -LEFTOVERS_TO_CLEAN=() - ignore_test_cases() { COMMAND_TO_RUN+=(-e FILTER_REGEX_EXCLUDE=".*(/test/linters/|CHANGELOG.md).*") } @@ -167,11 +165,16 @@ run_test_case_custom_summary() { run_test_case_fix_mode() { CREATE_LOG_FILE="true" SAVE_SUPER_LINTER_OUTPUT="true" + VERIFY_FIX_MODE="true" GIT_REPOSITORY_PATH="$(mktemp -d)" - initialize_git_repository_and_test_args "${GIT_REPOSITORY_PATH}" "test/data/github-event/github-event-push.json" + # Remove leftovers before copying test files because other tests might have + # created temporary files and caches as the root user, so commands that + # need access to those files might fail if they run as a non-root user. + RemoveTestLeftovers + local LINTERS_TEST_CASES_FIX_MODE_DESTINATION_PATH="${GIT_REPOSITORY_PATH}/${LINTERS_TEST_CASE_DIRECTORY}" mkdir -p "${LINTERS_TEST_CASES_FIX_MODE_DESTINATION_PATH}" @@ -200,10 +203,6 @@ run_test_case_fix_mode() { git -C "${GIT_REPOSITORY_PATH}" commit --no-verify -m "feat: add fix mode test cases" initialize_github_sha "${GIT_REPOSITORY_PATH}" - CREATE_LOG_FILE="true" - SAVE_SUPER_LINTER_OUTPUT="true" - VERIFY_FIX_MODE="true" - # Enable test mode so we run linters and formatters only against their test # cases COMMAND_TO_RUN+=(--env FIX_MODE_TEST_CASE_RUN=true) @@ -266,32 +265,14 @@ SUPER_LINTER_SUMMARY_FILE_PATH="${SUPER_LINTER_MAIN_OUTPUT_PATH}/${SUPER_LINTER_ debug "Super-linter summary output path: ${SUPER_LINTER_SUMMARY_FILE_PATH}" LOG_FILE_PATH="${SUPER_LINTER_WORKSPACE}/super-linter.log" +debug "Super-linter log file path: ${LOG_FILE_PATH}" COMMAND_TO_RUN+=("${SUPER_LINTER_TEST_CONTAINER_URL}") declare -i EXPECTED_EXIT_CODE EXPECTED_EXIT_CODE=${EXPECTED_EXIT_CODE:-0} -debug "Cleaning eventual leftovers before running tests: ${LEFTOVERS_TO_CLEAN[*]}" -LEFTOVERS_TO_CLEAN+=("${LOG_FILE_PATH}") -LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_GITHUB_STEP_SUMMARY_FILE_PATH}") -LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_MAIN_OUTPUT_PATH}") -LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_SUMMARY_FILE_PATH}") -LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_SUMMARY_FILE_PATH}") -LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_WORKSPACE}/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/bad/target") -LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_WORKSPACE}/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/bad/Cargo.lock") -LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_WORKSPACE}/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/good/target") -LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_WORKSPACE}/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/good/Cargo.lock") -# Delete leftovers in pwd in case the workspace is not pwd -LEFTOVERS_TO_CLEAN+=("$(pwd)/$(basename "${LOG_FILE_PATH}")") -LEFTOVERS_TO_CLEAN+=("$(pwd)/$(basename "${SUPER_LINTER_GITHUB_STEP_SUMMARY_FILE_PATH}")") -LEFTOVERS_TO_CLEAN+=("$(pwd)/$(basename "${SUPER_LINTER_MAIN_OUTPUT_PATH}")") -LEFTOVERS_TO_CLEAN+=("$(pwd)/$(basename "${SUPER_LINTER_SUMMARY_FILE_PATH}")") -LEFTOVERS_TO_CLEAN+=("$(pwd)/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/bad/target") -LEFTOVERS_TO_CLEAN+=("$(pwd)/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/bad/Cargo.lock") -LEFTOVERS_TO_CLEAN+=("$(pwd)/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/good/target") -LEFTOVERS_TO_CLEAN+=("$(pwd)/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/good/Cargo.lock") -sudo rm -rfv "${LEFTOVERS_TO_CLEAN[@]}" +RemoveTestLeftovers if [[ "${ENABLE_GITHUB_ACTIONS_STEP_SUMMARY}" == "true" ]]; then debug "Creating GitHub Actions step summary file: ${SUPER_LINTER_GITHUB_STEP_SUMMARY_FILE_PATH}" diff --git a/test/testUtils.sh b/test/testUtils.sh index 451245e1..3a4f59bd 100755 --- a/test/testUtils.sh +++ b/test/testUtils.sh @@ -123,3 +123,45 @@ IsLanguageInSlimImage() { return 0 fi } + +RemoveTestLeftovers() { + local LEFTOVERS_TO_CLEAN=() + LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_WORKSPACE}/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/bad/target") + LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_WORKSPACE}/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/bad/Cargo.lock") + LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_WORKSPACE}/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/good/target") + LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_WORKSPACE}/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/good/Cargo.lock") + # Delete leftovers in pwd in case the workspace is not pwd + LEFTOVERS_TO_CLEAN+=("$(pwd)/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/bad/target") + LEFTOVERS_TO_CLEAN+=("$(pwd)/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/bad/Cargo.lock") + LEFTOVERS_TO_CLEAN+=("$(pwd)/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/good/target") + LEFTOVERS_TO_CLEAN+=("$(pwd)/${LINTERS_TEST_CASE_DIRECTORY}/rust_clippy/good/Cargo.lock") + + # These variables are defined after configuring test cases, so they might not + # have been initialized yet + if [[ -v LOG_FILE_PATH ]] && + [[ -n "${LOG_FILE_PATH}" ]]; then + LEFTOVERS_TO_CLEAN+=("${LOG_FILE_PATH}") + LEFTOVERS_TO_CLEAN+=("$(pwd)/$(basename "${LOG_FILE_PATH}")") + fi + + if [[ -v SUPER_LINTER_GITHUB_STEP_SUMMARY_FILE_PATH ]] && + [[ -n "${SUPER_LINTER_GITHUB_STEP_SUMMARY_FILE_PATH}" ]]; then + LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_GITHUB_STEP_SUMMARY_FILE_PATH}") + LEFTOVERS_TO_CLEAN+=("$(pwd)/$(basename "${SUPER_LINTER_GITHUB_STEP_SUMMARY_FILE_PATH}")") + fi + + if [[ -v SUPER_LINTER_MAIN_OUTPUT_PATH ]] && + [[ -n "${SUPER_LINTER_MAIN_OUTPUT_PATH}" ]]; then + LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_MAIN_OUTPUT_PATH}") + LEFTOVERS_TO_CLEAN+=("$(pwd)/$(basename "${SUPER_LINTER_MAIN_OUTPUT_PATH}")") + fi + + if [[ -v SUPER_LINTER_SUMMARY_FILE_PATH ]] && + [[ -n "${SUPER_LINTER_SUMMARY_FILE_PATH}" ]]; then + LEFTOVERS_TO_CLEAN+=("${SUPER_LINTER_SUMMARY_FILE_PATH}") + LEFTOVERS_TO_CLEAN+=("$(pwd)/$(basename "${SUPER_LINTER_SUMMARY_FILE_PATH}")") + fi + + debug "Cleaning eventual test leftovers: ${LEFTOVERS_TO_CLEAN[*]}" + sudo rm -rfv "${LEFTOVERS_TO_CLEAN[@]}" +}