From 4f5775493d7464efc050fc23e142ce5fa448e78c Mon Sep 17 00:00:00 2001 From: Kyle Wiebers <kwiebers@gitlab.com> Date: Mon, 7 Dec 2020 16:01:07 +0000 Subject: [PATCH] Revert "Merge branch 'caalberts-use-crystalball-with-knapsack' into 'master'" This reverts merge request !49091 --- .gitlab/ci/rails.gitlab-ci.yml | 4 +-- .gitlab/ci/rules.gitlab-ci.yml | 5 +-- .gitlab/ci/setup.gitlab-ci.yml | 8 ++--- scripts/api/get_job_id | 35 +++------------------ scripts/rspec_helpers.sh | 28 ++++++----------- scripts/utils.sh | 2 +- tooling/bin/{find_tests => find_foss_tests} | 11 ++----- 7 files changed, 26 insertions(+), 67 deletions(-) rename tooling/bin/{find_tests => find_foss_tests} (58%) diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 9073bb231ccc..ba6c959d3ae6 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -22,7 +22,7 @@ RUBY_GC_MALLOC_LIMIT_MAX: 134217728 CRYSTALBALL: "true" RECORD_DEPRECATIONS: "true" - needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets", "detect-tests"] + needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets"] script: - *base-script - rspec_paralellized_job "--tag ~quarantine --tag ~geo --tag ~level:migration" @@ -64,7 +64,7 @@ - .rspec-base - .as-if-foss - .use-pg11 - needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets as-if-foss", "detect-tests"] + needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets as-if-foss"] .rspec-ee-base-pg11: extends: diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 419f2a289679..337816dede99 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -600,9 +600,10 @@ rules: - <<: *if-not-ee when: never - - <<: *if-default-refs + - <<: *if-security-merge-request + changes: *code-backstage-patterns + - <<: *if-dot-com-gitlab-org-merge-request changes: *code-backstage-patterns - - <<: *if-merge-request-title-run-all-rspec .rails:rules:rspec-foss-impact: rules: diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml index efbc3d47c829..abe7625c740f 100644 --- a/.gitlab/ci/setup.gitlab-ci.yml +++ b/.gitlab/ci/setup.gitlab-ci.yml @@ -61,16 +61,14 @@ verify-tests-yml: - scripts/verify-tff-mapping .detect-test-base: - image: ruby:2.7 + image: ruby:2.7-alpine needs: [] stage: prepare script: - - source ./scripts/utils.sh - - source ./scripts/rspec_helpers.sh + - source scripts/utils.sh - install_gitlab_gem - install_tff_gem - - retrieve_tests_mapping - - tooling/bin/find_tests ${MATCHED_TESTS_FILE} + - tooling/bin/find_foss_tests ${MATCHED_TESTS_FILE} - 'echo "test files affected: $(cat $MATCHED_TESTS_FILE)"' artifacts: expire_in: 7d diff --git a/scripts/api/get_job_id b/scripts/api/get_job_id index c7fe859db916..75ee9c548998 100755 --- a/scripts/api/get_job_id +++ b/scripts/api/get_job_id @@ -20,7 +20,6 @@ class JobFinder @job_query = options.delete(:job_query) @pipeline_id = options.delete(:pipeline_id) @job_name = options.delete(:job_name) - @artifact_path = options.delete(:artifact_path) # Force the token to be a string so that if api_token is nil, it's set to '', allowing unauthenticated requests (for forks). api_token = options.delete(:api_token).to_s @@ -34,31 +33,19 @@ class JobFinder end def execute - find_job_with_artifact || find_job_with_filtered_pipelines || find_job_in_pipeline + find_job_with_filtered_pipelines || find_job_in_pipeline end private - attr_reader :project, :pipeline_query, :job_query, :pipeline_id, :job_name, :artifact_path - - def find_job_with_artifact - return if artifact_path.nil? - - Gitlab.pipelines(project, pipeline_query_params).auto_paginate do |pipeline| - Gitlab.pipeline_jobs(project, pipeline.id, job_query_params).auto_paginate do |job| - return job if found_job_with_artifact?(job) # rubocop:disable Cop/AvoidReturnFromBlocks - end - end - - raise 'Job not found!' - end + attr_reader :project, :pipeline_query, :job_query, :pipeline_id, :job_name def find_job_with_filtered_pipelines return if pipeline_query.empty? Gitlab.pipelines(project, pipeline_query_params).auto_paginate do |pipeline| Gitlab.pipeline_jobs(project, pipeline.id, job_query_params).auto_paginate do |job| - return job if found_job_by_name?(job) # rubocop:disable Cop/AvoidReturnFromBlocks + return job if job.name == job_name # rubocop:disable Cop/AvoidReturnFromBlocks end end @@ -69,22 +56,12 @@ class JobFinder return unless pipeline_id Gitlab.pipeline_jobs(project, pipeline_id, job_query_params).auto_paginate do |job| - return job if found_job_by_name?(job) # rubocop:disable Cop/AvoidReturnFromBlocks + return job if job.name == job_name # rubocop:disable Cop/AvoidReturnFromBlocks end raise 'Job not found!' end - def found_job_with_artifact?(job) - artifact_url = "https://gitlab.com/api/v4/projects/#{CGI.escape(project)}/jobs/#{job.id}/artifacts/#{artifact_path}" - response = HTTParty.head(artifact_url) # rubocop:disable Gitlab/HTTParty - response.success? - end - - def found_job_by_name?(job) - job.name == job_name - end - def pipeline_query_params @pipeline_query_params ||= { per_page: 100, **pipeline_query } end @@ -118,10 +95,6 @@ if $0 == __FILE__ options[:job_name] = value end - opts.on("-a", "--artifact-path ARTIFACT_PATH", String, "A valid artifact path") do |value| - options[:artifact_path] = value - end - opts.on("-t", "--api-token API_TOKEN", String, "A value API token with the `read_api` scope") do |value| options[:api_token] = value end diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh index 5b724c9251bd..122f830ce45e 100644 --- a/scripts/rspec_helpers.sh +++ b/scripts/rspec_helpers.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash function retrieve_tests_metadata() { - mkdir -p knapsack/ rspec_flaky/ rspec_profiling/ + mkdir -p crystalball/ knapsack/ rspec_flaky/ rspec_profiling/ local project_path="gitlab-org/gitlab" local test_metadata_job_id @@ -16,6 +16,13 @@ function retrieve_tests_metadata() { if [[ ! -f "${FLAKY_RSPEC_SUITE_REPORT_PATH}" ]]; then scripts/api/download_job_artifact --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${FLAKY_RSPEC_SUITE_REPORT_PATH}" || echo "{}" > "${FLAKY_RSPEC_SUITE_REPORT_PATH}" fi + + # FIXME: We will need to find a pipeline where the $RSPEC_PACKED_TESTS_MAPPING_PATH.gz actually exists (Crystalball only runs every two-hours, but the `update-tests-metadata` runs for all `master` pipelines...). + # if [[ ! -f "${RSPEC_PACKED_TESTS_MAPPING_PATH}" ]]; then + # (scripts/api/download_job_artifact --project "${project_path}" --job-id "${test_metadata_job_id}" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" && gzip -d "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") || echo "{}" > "${RSPEC_PACKED_TESTS_MAPPING_PATH}" + # fi + # + # scripts/unpack-test-mapping "${RSPEC_PACKED_TESTS_MAPPING_PATH}" "${RSPEC_TESTS_MAPPING_PATH}" } function update_tests_metadata() { @@ -36,21 +43,6 @@ function update_tests_metadata() { fi } -function retrieve_tests_mapping() { - mkdir -p crystalball/ - - local project_path="gitlab-org/gitlab" - local test_metadata_with_mapping_job_id - - test_metadata_with_mapping_job_id=$(scripts/api/get_job_id --project "${project_path}" -q "status=success" -q "ref=master" -q "username=gitlab-bot" -Q "scope=success" --job-name "update-tests-metadata" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") - - if [[ ! -f "${RSPEC_PACKED_TESTS_MAPPING_PATH}" ]]; then - (scripts/api/download_job_artifact --project "${project_path}" --job-id "${test_metadata_with_mapping_job_id}" --artifact-path "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz" && gzip -d "${RSPEC_PACKED_TESTS_MAPPING_PATH}.gz") || echo "{}" > "${RSPEC_PACKED_TESTS_MAPPING_PATH}" - fi - - scripts/unpack-test-mapping "${RSPEC_PACKED_TESTS_MAPPING_PATH}" "${RSPEC_TESTS_MAPPING_PATH}" -} - function update_tests_mapping() { if ! crystalball_rspec_data_exists; then echo "No crystalball rspec data found." @@ -127,8 +119,8 @@ function rspec_paralellized_job() { local rspec_args="-Ispec -rspec_helper --color --format documentation --format RspecJunitFormatter --out junit_rspec.xml ${rspec_opts}" - if [[ -n $RSPEC_TESTS_MAPPING_ENABLED ]]; then - tooling/bin/parallel_rspec --rspec_args "${rspec_args}" --filter "tmp/matching_tests.txt" + if [[ -n $RSPEC_MATCHING_TESTS_ENABLED ]]; then + tooling/bin/parallel_rspec --rspec_args "${rspec_args}" --filter tmp/matching_tests.txt else tooling/bin/parallel_rspec --rspec_args "${rspec_args}" fi diff --git a/scripts/utils.sh b/scripts/utils.sh index 6747efa73d7b..4d6088e94a8d 100644 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -36,7 +36,7 @@ function install_gitlab_gem() { } function install_tff_gem() { - gem install test_file_finder --version 0.1.1 + gem install test_file_finder --version 0.1.0 } function run_timed_command() { diff --git a/tooling/bin/find_tests b/tooling/bin/find_foss_tests similarity index 58% rename from tooling/bin/find_tests rename to tooling/bin/find_foss_tests index 2c0e7ae2c53b..9cd8a616ad07 100755 --- a/tooling/bin/find_tests +++ b/tooling/bin/find_foss_tests @@ -19,12 +19,7 @@ mr_iid = ENV.fetch('CI_MERGE_REQUEST_IID') mr_changes = Gitlab.merge_request_changes(mr_project_path, mr_iid) changed_files = mr_changes.changes.map { |change| change['new_path'] } -tff = TestFileFinder::FileFinder.new(paths: changed_files).tap do |file_finder| - file_finder.use TestFileFinder::MappingStrategies::PatternMatching.load('tests.yml') +mapping = TestFileFinder::Mapping.load('tests.yml') +test_files = TestFileFinder::FileFinder.new(paths: changed_files, mapping: mapping).test_files - if ENV['RSPEC_TESTS_MAPPING_ENABLED'] - file_finder.use TestFileFinder::MappingStrategies::DirectMatching.load_json(ENV['RSPEC_TESTS_MAPPING_PATH']) - end -end - -File.write(output_file, tff.test_files.uniq.join(' ')) +File.write(output_file, test_files.uniq.join(' ')) -- GitLab