diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 21dfd6563e2..48412bae40c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -15,7 +15,7 @@ stages: # in cases where jobs require Docker-in-Docker, the job # definition must be extended with `.use-docker-in-docker` default: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6.6-golang-1.14-git-2.26-lfs-2.9-chrome-73.0-node-12.x-yarn-1.21-postgresql-9.6-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6.6-golang-1.14-git-2.26-lfs-2.9-chrome-73.0-node-12.x-yarn-1.21-postgresql-11-graphicsmagick-1.3.34" tags: - gitlab-org # All jobs are interruptible by default diff --git a/.gitlab/ci/docs.gitlab-ci.yml b/.gitlab/ci/docs.gitlab-ci.yml index 50dbef44598..5a6f2aacf93 100644 --- a/.gitlab/ci/docs.gitlab-ci.yml +++ b/.gitlab/ci/docs.gitlab-ci.yml @@ -62,7 +62,7 @@ docs lint: graphql-reference-verify: extends: - .default-retry - - .default-cache + - .rails-cache - .default-before_script - .docs:rules:graphql-reference-verify - .use-pg11 diff --git a/.gitlab/ci/frontend.gitlab-ci.yml b/.gitlab/ci/frontend.gitlab-ci.yml index 80eb067b4c8..79b4b8d48f1 100644 --- a/.gitlab/ci/frontend.gitlab-ci.yml +++ b/.gitlab/ci/frontend.gitlab-ci.yml @@ -1,156 +1,127 @@ -.assets-compile-cache: - cache: - paths: - - vendor/ruby/ - - public/assets/webpack/ - - assets-hash.txt - - .yarn-cache/ - - tmp/cache/assets/sprockets - - tmp/cache/babel-loader - - tmp/cache/vue-loader - - tmp/cache/webpack-dlls - -.gitlab:assets:compile-metadata: +.frontend-base: extends: - .default-retry - .default-before_script - .assets-compile-cache - image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6.6-git-2.26-lfs-2.9-chrome-73.0-node-12.x-yarn-1.21-graphicsmagick-1.3.34-docker-19.03.1 + variables: + SETUP_DB: "false" + # we override the max_old_space_size to prevent OOM errors + NODE_OPTIONS: --max_old_space_size=3584 + WEBPACK_VENDOR_DLL: "true" + +.compile-assets-base: + extends: .frontend-base + image: registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6.6-git-2.26-lfs-2.9-node-12.x-yarn-1.21-graphicsmagick-1.3.34 stage: prepare + script: + - node --version + - run_timed_command "retry yarn install --frozen-lockfile" + - free -m + - run_timed_command "bin/rake gitlab:assets:compile > assets-compile.log 2>&1" + - run_timed_command "scripts/clean-old-cached-assets" + +compile-production-assets: + extends: + - .compile-assets-base + - .frontend:rules:compile-production-assets variables: NODE_ENV: "production" RAILS_ENV: "production" - SETUP_DB: "false" WEBPACK_REPORT: "true" - # we override the max_old_space_size to prevent OOM errors - NODE_OPTIONS: --max_old_space_size=3584 - cache: - key: "assets-compile:production:v1" artifacts: name: webpack-report expire_in: 31d paths: - - webpack-report/ - assets-compile.log # These assets are used in multiple locations: # - in `build-assets-image` job to create assets image for packaging systems # - GitLab UI for integration tests: https://gitlab.com/gitlab-org/gitlab-ui/-/blob/e88493b3c855aea30bf60baee692a64606b0eb1e/.storybook/preview-head.pug#L1 - - public/assets + - public/assets/ + - webpack-report/ when: always - script: - - node --version - - retry yarn install --frozen-lockfile --production --cache-folder .yarn-cache --prefer-offline - - free -m - - time bin/rake gitlab:assets:compile > assets-compile.log 2>&1 - - scripts/clean-old-cached-assets + after_script: - rm -f /etc/apt/sources.list.d/google*.list # We don't need to update Chrome here -gitlab:assets:compile pull-push-cache: +compile-test-assets: extends: - - .gitlab:assets:compile-metadata - - .frontend:rules:gitlab-assets-compile-pull-push-cache - cache: - policy: pull-push + - .compile-assets-base + - .frontend:rules:compile-test-assets + artifacts: + expire_in: 7d + paths: + - assets-compile.log + - public/assets/ + - node_modules/@gitlab/svgs/dist/icons.json # app/helpers/icons_helper.rb uses this file + when: always -gitlab:assets:compile pull-cache: +compile-test-assets as-if-foss: extends: - - .gitlab:assets:compile-metadata - - .frontend:rules:gitlab-assets-compile-pull-cache + - compile-test-assets + - .frontend:rules:compile-test-assets-as-if-foss + - .as-if-foss + +update-assets-compile-production-cache: + extends: + - compile-production-assets + - .shared:rules:update-cache + stage: prepare + artifacts: {} # This job's purpose is only to update the cache. cache: - policy: pull + policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up. + +update-assets-compile-test-cache: + extends: + - compile-test-assets + - .shared:rules:update-cache + stage: prepare + artifacts: {} # This job's purpose is only to update the cache. + cache: + policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up. + +update-yarn-cache: + extends: + - .default-retry + - .yarn-cache + - .shared:rules:update-cache + stage: prepare + script: + - source scripts/utils.sh + - run_timed_command "retry yarn install --frozen-lockfile" + cache: + policy: push build-assets-image: extends: - .use-kaniko - - .frontend:rules:gitlab-assets-compile-pull-cache + - .frontend:rules:compile-production-assets stage: build-images - needs: ["gitlab:assets:compile pull-cache"] + needs: ["compile-production-assets"] variables: GIT_DEPTH: "1" script: # TODO: Change the image tag to be the MD5 of assets files and skip image building if the image exists # We'll also need to pass GITLAB_ASSETS_TAG to the trigerred omnibus-gitlab pipeline similarly to how we do it for trigerred CNG pipelines # https://gitlab.com/gitlab-org/gitlab/issues/208389 - - scripts/build_assets_image - -.compile-assets-metadata: - extends: - - .default-retry - - .default-before_script - - .assets-compile-cache - stage: prepare - script: - - node --version - - retry yarn install --frozen-lockfile --cache-folder .yarn-cache --prefer-offline - - free -m - - time bin/rake gitlab:assets:compile > assets-compile.log 2>&1 - - scripts/clean-old-cached-assets - variables: - SETUP_DB: "false" - # we override the max_old_space_size to prevent OOM errors - NODE_OPTIONS: --max_old_space_size=3584 - WEBPACK_VENDOR_DLL: "true" - cache: - key: "assets-compile:test:v1" - artifacts: - expire_in: 7d - paths: - - node_modules - - public/assets - - assets-compile.log - when: always - -compile-assets pull-push-cache: - extends: - - .compile-assets-metadata - - .frontend:rules:compile-assets-pull-push-cache - cache: - policy: pull-push - -compile-assets pull-push-cache as-if-foss: - extends: - - .compile-assets-metadata - - .frontend:rules:compile-assets-pull-push-cache-as-if-foss - - .as-if-foss - cache: - policy: pull-push - key: "assets-compile:test:as-if-foss:v1" - -compile-assets pull-cache: - extends: - - .compile-assets-metadata - - .frontend:rules:compile-assets-pull-cache - cache: - policy: pull - -compile-assets pull-cache as-if-foss: - extends: - - .compile-assets-metadata - - .frontend:rules:compile-assets-pull-cache-as-if-foss - - .as-if-foss - cache: - policy: pull - key: "assets-compile:test:as-if-foss:v1" + - run_timed_command "scripts/build_assets_image" .frontend-fixtures-base: extends: - - .default-retry + - .frontend-base - .rails-cache - - .default-before_script - .use-pg11 stage: fixtures - needs: ["setup-test-env", "compile-assets pull-cache"] + needs: ["setup-test-env", "compile-test-assets"] + variables: + SETUP_DB: "true" script: - run_timed_command "scripts/gitaly-test-build" - run_timed_command "scripts/gitaly-test-spawn" - - run_timed_command "bundle exec rake frontend:fixtures" + - run_timed_command "bin/rake frontend:fixtures" artifacts: name: frontend-fixtures expire_in: 31d when: always paths: - - node_modules - - public/assets - tmp/tests/frontend/ frontend-fixtures: @@ -164,25 +135,27 @@ frontend-fixtures-as-if-foss: - .frontend:rules:default-frontend-jobs-as-if-foss - .as-if-foss -.frontend-job-base: +.frontend-test-base: extends: - .default-retry - - .default-cache - - .default-before_script + - .yarn-cache variables: USE_BUNDLE_INSTALL: "false" SETUP_DB: "false" stage: test + before_script: + - source scripts/utils.sh .karma-base: - extends: .frontend-job-base + extends: .frontend-test-base variables: # we override the max_old_space_size to prevent OOM errors NODE_OPTIONS: --max_old_space_size=3584 script: + - source scripts/utils.sh - export BABEL_ENV=coverage CHROME_LOG_FILE=chrome_debug.log - - date - - yarn karma + - run_timed_command "retry yarn install --frozen-lockfile" + - run_timed_command "yarn karma" karma: extends: @@ -209,15 +182,11 @@ karma-as-if-foss: needs: ["frontend-fixtures-as-if-foss"] .jest-base: - extends: .frontend-job-base + extends: .frontend-test-base script: - - date - - yarn jest --ci --coverage --testSequencer ./scripts/frontend/parallel_ci_sequencer.js - cache: - key: jest - paths: - - tmp/cache/jest/ - policy: pull-push + - source scripts/utils.sh + - run_timed_command "retry yarn install --frozen-lockfile" + - run_timed_command "yarn jest --ci --coverage --testSequencer ./scripts/frontend/parallel_ci_sequencer.js" jest: extends: @@ -238,17 +207,13 @@ jest: jest-integration: extends: - - .frontend-job-base + - .frontend-test-base - .frontend:rules:default-frontend-jobs script: - - date - - yarn jest:integration --ci + - source scripts/utils.sh + - run_timed_command "retry yarn install --frozen-lockfile" + - run_timed_command "yarn jest:integration --ci" needs: ["frontend-fixtures"] - cache: - key: jest-integration - paths: - - tmp/cache/jest/ - policy: pull-push jest-as-if-foss: extends: @@ -256,8 +221,6 @@ jest-as-if-foss: - .frontend:rules:default-frontend-jobs-as-if-foss - .as-if-foss needs: ["frontend-fixtures-as-if-foss"] - cache: - policy: pull coverage-frontend: extends: @@ -268,33 +231,26 @@ coverage-frontend: stage: post-test before_script: - source scripts/utils.sh - - retry yarn install --frozen-lockfile + - run_timed_command "retry yarn install --frozen-lockfile" script: - - yarn node scripts/frontend/merge_coverage_frontend.js + - run_timed_command "yarn node scripts/frontend/merge_coverage_frontend.js" artifacts: name: coverage-frontend expire_in: 31d paths: - coverage-frontend/ - cache: - policy: pull .qa-frontend-node: extends: - .default-retry + - .yarn-cache - .frontend:rules:qa-frontend-node stage: test dependencies: [] - cache: - key: "$CI_JOB_NAME" - paths: - - .yarn-cache/ - policy: pull-push script: - - date - - yarn install --frozen-lockfile --cache-folder .yarn-cache --prefer-offline - - date - - yarn run webpack-prod + - source scripts/utils.sh + - run_timed_command "yarn install --frozen-lockfile" + - run_timed_command "yarn run webpack-prod" qa-frontend-node:10: extends: .qa-frontend-node @@ -309,25 +265,18 @@ qa-frontend-node:latest: webpack-dev-server: extends: - .default-retry + - .yarn-cache - .frontend:rules:default-frontend-jobs stage: test needs: [] variables: WEBPACK_MEMORY_TEST: "true" WEBPACK_VENDOR_DLL: "true" - cache: - key: - files: - - yarn.lock - prefix: "v1" - paths: - - node_modules/ - - tmp/cache/webpack-dlls/ script: - source scripts/utils.sh - - retry yarn install --frozen-lockfile - - retry yarn webpack-vendor - - node --expose-gc node_modules/.bin/webpack-dev-server --config config/webpack.config.js + - run_timed_command "retry yarn install --frozen-lockfile" + - run_timed_command "retry yarn webpack-vendor" + - run_timed_command "node --expose-gc node_modules/.bin/webpack-dev-server --config config/webpack.config.js" artifacts: name: webpack-dev-server expire_in: 31d @@ -340,7 +289,7 @@ bundle-size-review: - .frontend:rules:bundle-size-review image: registry.gitlab.com/gitlab-org/gitlab-build-images:danger stage: test - needs: ["gitlab:assets:compile pull-cache"] + needs: ["compile-production-assets"] script: - mkdir -p bundle-size-review - cp webpack-report/index.html bundle-size-review/bundle-report.html diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml index e6619ff2b6d..bf6c2ff18e6 100644 --- a/.gitlab/ci/global.gitlab-ci.yml +++ b/.gitlab/ci/global.gitlab-ci.yml @@ -10,46 +10,58 @@ .default-before_script: before_script: - - date - '[ "$FOSS_ONLY" = "1" ] && rm -rf ee/ qa/spec/ee/ qa/qa/specs/features/ee/ qa/qa/ee/ qa/qa/ee.rb' - export GOPATH=$CI_PROJECT_DIR/.go - mkdir -p $GOPATH - source scripts/utils.sh - source scripts/prepare_build.sh - - date - -# Jobs that only need to pull cache -.default-cache: - cache: - key: "debian-stretch-ruby-2.6.6-pg11-node-12.x" - paths: - - .go/pkg/mod - - vendor/ruby - - .yarn-cache/ - - vendor/gitaly-ruby - policy: pull .rails-cache: cache: - key: - files: - - Gemfile.lock - - GITALY_SERVER_VERSION - prefix: "ruby-go-cache-v1" + key: "rails-v1" paths: - - vendor/ruby - - vendor/gitaly-ruby - - .go/pkg/mod + - vendor/ruby/ + - vendor/gitaly-ruby/ + - .go/pkg/mod/ + policy: pull + +.static-analysis-cache: + cache: + key: "static-analysis-v1" + paths: + - vendor/ruby/ + - node_modules/ + - tmp/rubocop_cache/ + policy: pull + +.qa-cache: + cache: + key: "qa-v1" + paths: + - qa/vendor/ruby/ policy: pull .yarn-cache: cache: - key: - files: - - yarn.lock - prefix: "v1" + key: "yarn-v1" paths: - node_modules/ + - tmp/cache/webpack-dlls/ + policy: pull + +.assets-compile-cache: + cache: + key: "assets-compile-${NODE_ENV}-v1" + paths: + - vendor/ruby/ + - node_modules/ + - assets-hash.txt + - public/assets/webpack/ + - tmp/cache/assets/sprockets/ + - tmp/cache/babel-loader/ + - tmp/cache/vue-loader/ + - tmp/cache/webpack-dlls/ + policy: pull .use-pg11: image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6.6-golang-1.14-git-2.26-lfs-2.9-chrome-73.0-node-12.x-yarn-1.21-postgresql-11-graphicsmagick-1.3.34" @@ -75,6 +87,7 @@ name: gcr.io/kaniko-project/executor:debug-v0.20.0 entrypoint: [""] before_script: + - source scripts/utils.sh - mkdir -p /kaniko/.docker - echo "{\"auths\":{\"$CI_REGISTRY\":{\"username\":\"$CI_REGISTRY_USER\",\"password\":\"$CI_REGISTRY_PASSWORD\"}}}" > /kaniko/.docker/config.json diff --git a/.gitlab/ci/memory.gitlab-ci.yml b/.gitlab/ci/memory.gitlab-ci.yml index 4d5c0d1c902..ef6c9b9c8ff 100644 --- a/.gitlab/ci/memory.gitlab-ci.yml +++ b/.gitlab/ci/memory.gitlab-ci.yml @@ -1,7 +1,7 @@ .only-code-memory-job-base: extends: - .default-retry - - .default-cache + - .rails-cache - .default-before_script - .memory:rules @@ -39,7 +39,7 @@ memory-on-boot: - .only-code-memory-job-base - .use-pg11 stage: test - needs: ["setup-test-env", "compile-assets pull-cache"] + needs: ["setup-test-env", "compile-test-assets"] variables: NODE_ENV: "production" RAILS_ENV: "production" diff --git a/.gitlab/ci/pages.gitlab-ci.yml b/.gitlab/ci/pages.gitlab-ci.yml index d374f53a5e2..a66e0d88db3 100644 --- a/.gitlab/ci/pages.gitlab-ci.yml +++ b/.gitlab/ci/pages.gitlab-ci.yml @@ -7,7 +7,7 @@ pages: - rspec:coverage - coverage-frontend - karma - - gitlab:assets:compile pull-cache + - compile-production-assets script: - mv public/ .public/ - mkdir public/ diff --git a/.gitlab/ci/qa.gitlab-ci.yml b/.gitlab/ci/qa.gitlab-ci.yml index 40ef13dd92b..20527b690a7 100644 --- a/.gitlab/ci/qa.gitlab-ci.yml +++ b/.gitlab/ci/qa.gitlab-ci.yml @@ -1,12 +1,9 @@ .qa-job-base: extends: - .default-retry + - .qa-cache stage: test needs: [] - cache: - key: "qa-framework-jobs:v1" - paths: - - vendor/ruby before_script: - '[ "$FOSS_ONLY" = "1" ] && rm -rf ee/ qa/spec/ee/ qa/qa/specs/features/ee/ qa/qa/ee/ qa/qa/ee.rb' - cd qa/ @@ -22,11 +19,9 @@ qa:internal: qa:internal-as-if-foss: extends: - - .qa-job-base + - qa:internal - .qa:rules:as-if-foss - .as-if-foss - script: - - bundle exec rspec qa:selectors: extends: @@ -41,6 +36,16 @@ qa:selectors-as-if-foss: - .qa:rules:as-if-foss - .as-if-foss +update-qa-cache: + extends: + - .qa-job-base + - .shared:rules:update-cache + stage: prepare + script: + - echo "Cache has been updated and ready to be uploaded." + cache: + policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up. + .package-and-qa-base: image: ruby:2.6-alpine stage: qa diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index fa468634c33..07dbf6e37a4 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -1,6 +1,3 @@ -.rails:needs:setup-and-assets: - needs: ["setup-test-env", "compile-assets pull-cache"] - .rails-job-base: extends: - .default-retry @@ -35,33 +32,54 @@ setup-test-env: - tmp/tests/repositories - tmp/tests/second_storage when: always + +update-rails-cache: + extends: + - setup-test-env + - .shared:rules:update-cache + artifacts: {} # This job's purpose is only to update the cache. cache: + policy: push # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up. + +.static-analysis-base: + extends: + - .default-retry + - .default-before_script + - .static-analysis-cache + needs: [] + variables: + SETUP_DB: "false" + ENABLE_SPRING: "1" + +update-static-analysis-cache: + extends: + - .static-analysis-base + - .shared:rules:update-cache + stage: prepare + script: + - rm -rf ./node_modules # We remove node_modules because there's no mechanism to remove stall entries. + - run_timed_command "retry yarn install --frozen-lockfile" + - bundle exec rubocop --parallel # For the moment we only cache `vendor/ruby/`, `node_modules/`, and `tmp/rubocop_cache` so we don't need to run all the tasks, + cache: + # We want to rebuild the cache from scratch to ensure stale dependencies are cleaned up but RuboCop has a mechanism + # for keeping only the N latest cache files, so we take advantage of it with `pull-push` and removing `node_modules` at the start of the job. policy: pull-push static-analysis: extends: - - .rails-job-base + - .static-analysis-base - .rails:rules:default-refs-code-backstage-qa - - .rails:needs:setup-and-assets stage: test - variables: - SETUP_DB: "false" - ENABLE_SPRING: "1" parallel: 4 script: + - run_timed_command "retry yarn install --frozen-lockfile" - scripts/static-analysis - cache: - key: "ruby-2.6.6-pg11-rubocop" - paths: - - vendor/ruby - - tmp/rubocop_cache - policy: pull-push downtime_check: extends: - .rails-job-base - .rails:rules:downtime_check - needs: ["setup-test-env"] + needs: [] stage: test variables: SETUP_DB: "false" @@ -71,7 +89,7 @@ downtime_check: .rspec-base: extends: .rails-job-base stage: test - needs: ["setup-test-env", "retrieve-tests-metadata", "compile-assets pull-cache"] + needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets"] script: - run_timed_command "scripts/gitaly-test-build" - run_timed_command "scripts/gitaly-test-spawn" @@ -219,8 +237,6 @@ rspec:coverage: - memory-on-boot variables: SETUP_DB: "false" - cache: - policy: pull script: - bundle exec scripts/merge-simplecov - bundle exec scripts/gather-test-memory-data @@ -248,7 +264,7 @@ rspec:coverage: - .rails:rules:as-if-foss - .as-if-foss - .use-pg11 - needs: ["setup-test-env", "retrieve-tests-metadata", "compile-assets pull-cache as-if-foss"] + needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets as-if-foss"] .rspec-ee-base-pg11: extends: diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 46a281cd48f..dbf7bd604ab 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -38,7 +38,7 @@ review-build-cng: - install_api_client_dependencies_with_apk - install_gitlab_gem needs: - - job: gitlab:assets:compile pull-cache + - job: compile-production-assets artifacts: false script: - BUILD_TRIGGER_TOKEN=$REVIEW_APPS_BUILD_TRIGGER_TOKEN ./scripts/trigger-build cng @@ -238,5 +238,3 @@ danger-review: - source scripts/utils.sh - retry yarn install --frozen-lockfile - danger --fail-on-errors=true --verbose - cache: - policy: pull diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index db5e9f8acf4..19f6e6f1346 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -37,6 +37,9 @@ .if-merge-request-title-as-if-foss: &if-merge-request-title-as-if-foss if: '$CI_MERGE_REQUEST_TITLE =~ /RUN AS-IF-FOSS/' +.if-merge-request-title-update-caches: &if-merge-request-title-update-caches + if: '$CI_MERGE_REQUEST_TITLE =~ /UPDATE CACHE/' + .if-security-merge-request: &if-security-merge-request if: '$CI_PROJECT_NAMESPACE == "gitlab-org/security" && $CI_MERGE_REQUEST_IID' @@ -183,6 +186,14 @@ - ".dockerignore" - "qa/**/*" +################ +# Shared rules # +################ +.shared:rules:update-cache: + rules: + - <<: *if-master-schedule-2-hourly + - <<: *if-merge-request-title-update-caches + #################### # Cache repo rules # #################### @@ -244,45 +255,18 @@ ################## # Frontend rules # ################## -# This job only runs on `master` since it pushes to the cache. -.frontend:rules:gitlab-assets-compile-pull-push-cache: - rules: - - <<: *if-not-canonical-namespace - when: never - - <<: *if-master-refs - changes: *code-backstage-qa-patterns - when: on_success - -.frontend:rules:gitlab-assets-compile-pull-cache: +.frontend:rules:compile-production-assets: rules: - <<: *if-not-canonical-namespace when: never - <<: *if-default-refs changes: *code-backstage-qa-patterns - when: on_success -.frontend:rules:compile-assets-pull-push-cache: +.frontend:rules:compile-test-assets: rules: - - <<: *if-master-refs - changes: *code-backstage-qa-patterns - when: on_success + - changes: *code-backstage-qa-patterns -# This job only runs on `master` since it pushes to the cache. -.frontend:rules:compile-assets-pull-push-cache-as-if-foss: - rules: - - <<: *if-not-ee - when: never - - <<: *if-master-push - changes: *code-backstage-qa-patterns - - <<: *if-master-schedule-2-hourly - -.frontend:rules:compile-assets-pull-cache: - rules: - - <<: *if-default-refs - changes: *code-backstage-qa-patterns - when: on_success - -.frontend:rules:compile-assets-pull-cache-as-if-foss: +.frontend:rules:compile-test-assets-as-if-foss: rules: - <<: *if-not-ee when: never @@ -299,7 +283,6 @@ rules: - <<: *if-default-refs changes: *code-backstage-patterns - when: on_success .frontend:rules:default-frontend-jobs-as-if-foss: rules: @@ -327,10 +310,8 @@ rules: - <<: *if-master-refs changes: *frontend-dependency-patterns - when: on_success - <<: *if-merge-request changes: *frontend-dependency-patterns - when: on_success .frontend:rules:qa-frontend-node-latest: rules: diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml index 9be495f1ef2..b878bec3751 100644 --- a/.gitlab/ci/setup.gitlab-ci.yml +++ b/.gitlab/ci/setup.gitlab-ci.yml @@ -3,7 +3,7 @@ cache gems: extends: - .default-retry - - .default-cache + - .rails-cache - .default-before_script - .setup:rules:cache-gems stage: test diff --git a/.gitlab/ci/test-metadata.gitlab-ci.yml b/.gitlab/ci/test-metadata.gitlab-ci.yml index 65cce76fc48..1764e9136a1 100644 --- a/.gitlab/ci/test-metadata.gitlab-ci.yml +++ b/.gitlab/ci/test-metadata.gitlab-ci.yml @@ -3,11 +3,6 @@ TESTS_METADATA_S3_BUCKET: "gitlab-ce-cache" before_script: - source scripts/utils.sh - cache: - key: tests_metadata - paths: - - knapsack/ - - rspec_flaky/ artifacts: expire_in: 31d paths: @@ -20,8 +15,6 @@ retrieve-tests-metadata: - .tests-metadata-state - .test-metadata:rules:retrieve-tests-metadata stage: prepare - cache: - policy: pull script: - source scripts/rspec_helpers.sh - retrieve_tests_metadata @@ -44,8 +37,6 @@ update-tests-metadata: - rspec-ee unit pg11 geo - rspec-ee integration pg11 geo - rspec-ee system pg11 geo - cache: - policy: push script: - retry gem install fog-aws mime-types activesupport rspec_profiling postgres-copy --no-document - source scripts/rspec_helpers.sh diff --git a/.rubocop.yml b/.rubocop.yml index aafcc4c7d9e..227c8dfa494 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -27,6 +27,7 @@ AllCops: - 'plugins/**/*' - 'file_hooks/**/*' CacheRootDirectory: tmp + MaxFilesInCache: 18000 Cop/AvoidKeywordArgumentsInSidekiqWorkers: Enabled: true @@ -274,6 +275,9 @@ Cop/ActiveRecordAssociationReload: - 'spec/**/*' - 'ee/spec/**/*' +Gitlab/AvoidFeatureGet: + Enabled: true + Naming/PredicateName: Enabled: true Exclude: diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index d8d8cc450f2..a8d348e1836 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -118,12 +118,7 @@ export const fetchDiffFilesBatch = ({ commit, state }) => { const getBatch = (page = 1) => axios - .get(state.endpointBatch, { - params: { - ...urlParams, - page, - }, - }) + .get(mergeUrlParams({ ...urlParams, page }, state.endpointBatch)) .then(({ data: { pagination, diff_files } }) => { commit(types.SET_DIFF_DATA_BATCH, { diff_files }); commit(types.SET_BATCH_LOADING, false); diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 1795a0dbdf8..6c63ab7cf95 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -126,6 +126,13 @@ export default class MergeRequestTabs { bindEvents() { $('.merge-request-tabs a[data-toggle="tabvue"]').on('click', this.clickTab); + window.addEventListener('popstate', event => { + if (event.state && event.state.action) { + this.tabShown(event.state.action, event.target.location); + this.currentAction = event.state.action; + this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction()); + } + }); } // Used in tests @@ -155,6 +162,12 @@ export default class MergeRequestTabs { } else if (action) { const href = e.currentTarget.getAttribute('href'); this.tabShown(action, href); + + if (this.setUrl) { + this.setCurrentAction(action); + } + + this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction()); } } } @@ -213,11 +226,6 @@ export default class MergeRequestTabs { this.resetViewContainer(); this.destroyPipelinesView(); } - if (this.setUrl) { - this.setCurrentAction(action); - } - - this.eventHub.$emit('MergeRequestTabChange', this.getCurrentAction()); } else if (action === this.currentAction) { // ContentTop is used to handle anything at the top of the page before the main content const mainContentContainer = document.querySelector('.content-wrapper'); @@ -287,19 +295,25 @@ export default class MergeRequestTabs { // Ensure parameters and hash come along for the ride newState += location.search + location.hash; - // TODO: Consider refactoring in light of turbolinks removal. - - // Replace the current history state with the new one without breaking - // Turbolinks' history. - // - // See https://github.com/rails/turbolinks/issues/363 - window.history.replaceState( - { - url: newState, - }, - document.title, - newState, - ); + if (window.history.state && window.history.state.url && window.location.pathname !== newState) { + window.history.pushState( + { + url: newState, + action: this.currentAction, + }, + document.title, + newState, + ); + } else { + window.history.replaceState( + { + url: window.location.href, + action, + }, + document.title, + window.location.href, + ); + } return newState; } diff --git a/app/controllers/concerns/integrations_actions.rb b/app/controllers/concerns/integrations_actions.rb index b06b7b212e9..cc9db7936e8 100644 --- a/app/controllers/concerns/integrations_actions.rb +++ b/app/controllers/concerns/integrations_actions.rb @@ -36,6 +36,10 @@ module IntegrationsActions end end + def custom_integration_projects + Project.with_custom_integration_compared_to(integration).page(params[:page]).per(20) + end + def test render json: {}, status: :ok end diff --git a/app/helpers/auto_devops_helper.rb b/app/helpers/auto_devops_helper.rb index 0f0d5350df6..0f14680607e 100644 --- a/app/helpers/auto_devops_helper.rb +++ b/app/helpers/auto_devops_helper.rb @@ -2,7 +2,7 @@ module AutoDevopsHelper def show_auto_devops_callout?(project) - Feature.get(:auto_devops_banner_disabled).off? && + Feature.disabled?(:auto_devops_banner_disabled) && show_callout?('auto_devops_settings_dismissed') && can?(current_user, :admin_pipeline, project) && project.has_auto_devops_implicitly_disabled? && diff --git a/app/models/concerns/integration.rb b/app/models/concerns/integration.rb new file mode 100644 index 00000000000..4a48a60803b --- /dev/null +++ b/app/models/concerns/integration.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Integration + extend ActiveSupport::Concern + + class_methods do + def with_custom_integration_compared_to(integration) + custom_integrations = Service + .select('1') + .where(type: integration.type, inherit_from_id: nil) + .where('services.project_id = projects.id') + + Project.where('EXISTS (?)', custom_integrations) + end + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 3f350c3fe1a..2d564c8c81d 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -33,6 +33,7 @@ class Project < ApplicationRecord include OptionallySearch include FromUnion include IgnorableColumns + include Integration extend Gitlab::Cache::RequestCache extend Gitlab::ConfigHelper @@ -902,17 +903,6 @@ class Project < ApplicationRecord latest_jira_import&.status || 'initial' end - def validate_jira_import_settings!(user: nil) - raise Projects::ImportService::Error, _('Jira integration not configured.') unless jira_service&.active? - - if user - raise Projects::ImportService::Error, _('Cannot import because issues are not available in this project.') unless feature_available?(:issues, user) - raise Projects::ImportService::Error, _('You do not have permissions to run the import.') unless user.can?(:admin_project, self) - end - - raise Projects::ImportService::Error, _('Unable to connect to the Jira instance. Please check your Jira integration configuration.') unless jira_service.test(nil)[:success] - end - def human_import_status_name import_state&.human_status_name || 'none' end diff --git a/app/services/discussions/resolve_service.rb b/app/services/discussions/resolve_service.rb index 3aa6245b54d..946fb5f1372 100644 --- a/app/services/discussions/resolve_service.rb +++ b/app/services/discussions/resolve_service.rb @@ -7,6 +7,7 @@ module Discussions def initialize(project, user = nil, params = {}) @discussions = Array.wrap(params.fetch(:one_or_more_discussions)) @follow_up_issue = params[:follow_up_issue] + @resolved_count = 0 raise ArgumentError, 'Discussions must be all for the same noteable' \ unless noteable_is_same? @@ -16,6 +17,7 @@ module Discussions def execute discussions.each(&method(:resolve_discussion)) + process_auto_merge end private @@ -36,6 +38,7 @@ module Discussions return unless discussion.can_resolve?(current_user) discussion.resolve!(current_user) + @resolved_count += 1 MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) if merge_request SystemNoteService.discussion_continued_in_issue(discussion, project, current_user, follow_up_issue) if follow_up_issue @@ -50,5 +53,17 @@ module Discussions first_discussion.noteable if first_discussion.for_merge_request? end end + + def process_auto_merge + return unless merge_request + return unless @resolved_count.positive? + return unless discussions_ready_to_merge? + + AutoMergeProcessWorker.perform_async(merge_request.id) + end + + def discussions_ready_to_merge? + merge_request.auto_merge_enabled? && merge_request.mergeable_discussions_state? + end end end diff --git a/app/services/jira_import/start_import_service.rb b/app/services/jira_import/start_import_service.rb index 615d06c7c3d..a06cc6df719 100644 --- a/app/services/jira_import/start_import_service.rb +++ b/app/services/jira_import/start_import_service.rb @@ -62,7 +62,7 @@ module JiraImport end def validate - project.validate_jira_import_settings!(user: user) + Gitlab::JiraImport.validate_project_settings!(project, user: user) return build_error_response(_('Unable to find Jira project to import data from.')) if jira_project_key.blank? return build_error_response(_('Jira import is already running.')) if import_in_progress? diff --git a/app/services/jira_import/users_importer.rb b/app/services/jira_import/users_importer.rb index 6976e2d570c..b7644ca54eb 100644 --- a/app/services/jira_import/users_importer.rb +++ b/app/services/jira_import/users_importer.rb @@ -13,7 +13,7 @@ module JiraImport end def execute - project.validate_jira_import_settings!(user: user) + Gitlab::JiraImport.validate_project_settings!(project, user: user) return ServiceResponse.success(payload: nil) if users.blank? diff --git a/changelogs/unreleased/add_build_reference.yml b/changelogs/unreleased/add_build_reference.yml new file mode 100644 index 00000000000..70db43c810d --- /dev/null +++ b/changelogs/unreleased/add_build_reference.yml @@ -0,0 +1,5 @@ +--- +title: Added build_id column to requirements_management_test_reports table +merge_request: 33184 +author: +type: other diff --git a/changelogs/unreleased/fix_back_button_when_switching_mr_tabs.yml b/changelogs/unreleased/fix_back_button_when_switching_mr_tabs.yml new file mode 100644 index 00000000000..bb9904faa50 --- /dev/null +++ b/changelogs/unreleased/fix_back_button_when_switching_mr_tabs.yml @@ -0,0 +1,5 @@ +--- +title: Fix back button when switching MR tabs +merge_request: 29862 +author: Lee Tickett +type: fixed diff --git a/changelogs/unreleased/sh-fix-auto-merge-after-resolve-discussions.yml b/changelogs/unreleased/sh-fix-auto-merge-after-resolve-discussions.yml new file mode 100644 index 00000000000..c4aa8d22ca3 --- /dev/null +++ b/changelogs/unreleased/sh-fix-auto-merge-after-resolve-discussions.yml @@ -0,0 +1,5 @@ +--- +title: Fix auto-merge not running after discussions resolved +merge_request: 33371 +author: +type: fixed diff --git a/changelogs/unreleased/switch-diff-view-fix.yml b/changelogs/unreleased/switch-diff-view-fix.yml new file mode 100644 index 00000000000..ff3e1e11e49 --- /dev/null +++ b/changelogs/unreleased/switch-diff-view-fix.yml @@ -0,0 +1,6 @@ +--- +title: Deduplicate URL parameters when requesting merge request diffs which causes + diffs load to fail +merge_request: 33117 +author: +type: fixed diff --git a/config/initializers/01_secret_token.rb b/config/initializers/01_secret_token.rb index e3fefeab81c..8b96727a2a1 100644 --- a/config/initializers/01_secret_token.rb +++ b/config/initializers/01_secret_token.rb @@ -61,6 +61,8 @@ def generate_new_rsa_private_key end def warn_missing_secret(secret) + return if Rails.env.test? + warn "Missing Rails.application.secrets.#{secret} for #{Rails.env} environment. The secret will be generated and stored in config/secrets.yml." end diff --git a/config/routes/admin.rb b/config/routes/admin.rb index f3b7fb5ed45..fa357220d2f 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -118,6 +118,7 @@ namespace :admin do resources :services, only: [:index, :edit, :update] resources :integrations, only: [:edit, :update] do member do + get :custom_integration_projects put :test end end diff --git a/db/migrate/20200527135313_add_requirements_build_reference.rb b/db/migrate/20200527135313_add_requirements_build_reference.rb new file mode 100644 index 00000000000..3385243fbdd --- /dev/null +++ b/db/migrate/20200527135313_add_requirements_build_reference.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddRequirementsBuildReference < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_requirements_management_test_reports_on_build_id' + + def up + add_column :requirements_management_test_reports, :build_id, :bigint + add_index :requirements_management_test_reports, :build_id, name: INDEX_NAME # rubocop:disable Migration/AddIndex + + with_lock_retries do + add_foreign_key :requirements_management_test_reports, :ci_builds, column: :build_id, on_delete: :nullify # rubocop:disable Migration/AddConcurrentForeignKey + end + end + + def down + with_lock_retries do + remove_column :requirements_management_test_reports, :build_id + end + end +end diff --git a/db/structure.sql b/db/structure.sql index a6e81d38141..11a4efec191 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5831,7 +5831,8 @@ CREATE TABLE public.requirements_management_test_reports ( requirement_id bigint NOT NULL, pipeline_id bigint, author_id bigint, - state smallint NOT NULL + state smallint NOT NULL, + build_id bigint ); CREATE SEQUENCE public.requirements_management_test_reports_id_seq @@ -10611,6 +10612,8 @@ CREATE UNIQUE INDEX index_repository_languages_on_project_and_languages_id ON pu CREATE INDEX index_requirements_management_test_reports_on_author_id ON public.requirements_management_test_reports USING btree (author_id); +CREATE INDEX index_requirements_management_test_reports_on_build_id ON public.requirements_management_test_reports USING btree (build_id); + CREATE INDEX index_requirements_management_test_reports_on_pipeline_id ON public.requirements_management_test_reports USING btree (pipeline_id); CREATE INDEX index_requirements_management_test_reports_on_requirement_id ON public.requirements_management_test_reports USING btree (requirement_id); @@ -12660,6 +12663,9 @@ ALTER TABLE ONLY public.approval_merge_request_rule_sources ALTER TABLE ONLY public.prometheus_alerts ADD CONSTRAINT fk_rails_e6351447ec FOREIGN KEY (prometheus_metric_id) REFERENCES public.prometheus_metrics(id) ON DELETE CASCADE; +ALTER TABLE ONLY public.requirements_management_test_reports + ADD CONSTRAINT fk_rails_e67d085910 FOREIGN KEY (build_id) REFERENCES public.ci_builds(id) ON DELETE SET NULL; + ALTER TABLE ONLY public.merge_request_metrics ADD CONSTRAINT fk_rails_e6d7c24d1b FOREIGN KEY (merge_request_id) REFERENCES public.merge_requests(id) ON DELETE CASCADE; @@ -14038,6 +14044,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200527092027 20200527094322 20200527095401 +20200527135313 20200527151413 20200527152116 20200527152657 diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 695de61f9a7..cad05ee5fc2 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -43,6 +43,7 @@ The following metrics are available: | `gitlab_cache_operation_duration_seconds` | Histogram | 10.2 | Cache access time | | | `gitlab_cache_operations_total` | Counter | 12.2 | Cache operations by controller/action | `controller`, `action`, `operation` | | `gitlab_ci_pipeline_creation_duration_seconds` | Histogram | 13.0 | Time in seconds it takes to create a CI/CD pipeline | | +| `gitlab_ci_pipeline_size_builds` | Histogram | 13.1 | Total number of builds within a pipeline grouped by a pipeline source | `source` | | `job_waiter_started_total` | Counter | 12.9 | Number of batches of jobs started where a web request is waiting for the jobs to complete | `worker` | | `job_waiter_timeouts_total` | Counter | 12.9 | Number of batches of jobs that timed out where a web request is waiting for the jobs to complete | `worker` | | `gitlab_database_transaction_seconds` | Histogram | 12.1 | Time spent in database transactions, in seconds | | diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 9ebcd70d1f9..7927bd9c86f 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -787,6 +787,131 @@ found, we should raise a `Gitlab::Graphql::Errors::ResourceNotAvailable` error. Which will be correctly rendered to the clients. +### Errors in mutations + +We encourage following the practice of [errors as +data](https://graphql-ruby.org/mutations/mutation_errors) for mutations, which +distinguishes errors by who they are relevant to, defined by who can deal with +them. + +Key points: + +- All mutation responses have an `errors` field. This should be populated on + failure, and may be populated on success. +- Consider who needs to see the error: the **user** or the **developer**. +- Clients should always request the `errors` field when performing mutations. +- Errors may be reported to users either at `$root.errors` (top-level error) or at + `$root.data.mutationName.errors` (mutation errors). The location depends on what kind of error + this is, and what information it holds. + +Consider an example mutation `doTheThing` that returns a response with +two fields: `errors: [String]`, and `thing: ThingType`. The specific nature of +the `thing` itself is irrelevant to these examples, as we are considering the +errors. + +There are three states a mutation response can be in: + +- [Success](#success) +- [Failure (relevant to the user)](#failure-relevant-to-the-user) +- [Failure (irrelevant to the user)](#failure-irrelevant-to-the-user) + +#### Success + +In the happy path, errors *may* be returned, along with the anticipated payload, but +if everything was successful, then `errors` should be an empty array, since +there are no problems we need to inform the user of. + +```javascript +{ + data: { + doTheThing: { + errors: [] // if successful, this array will generally be empty. + thing: { .. } + } + } +} +``` + +#### Failure (relevant to the user) + +An error that affects the **user** occurred. We refer to these as _mutation errors_. In +this case there is typically no `thing` to return: + +```javascript +{ + data: { + doTheThing: { + errors: ["you cannot touch the thing"], + thing: null + } + } +} +``` + +Examples of this include: + +- Model validation errors: the user may need to change the inputs. +- Permission errors: the user needs to know they cannot do this, they may need to request permission or sign in. +- Problems with application state that prevent the user's action, for example: merge conflicts, the resource was locked, and so on. + +Ideally, we should prevent the user from getting this far, but if they do, they +need to be told what is wrong, so they understand the reason for the failure and +what they can do to achieve their intent, even if that is as simple as retrying the +request. + +It is possible to return *recoverable* errors alongside mutation data. For example, if +a user uploads 10 files and 3 of them fail and the rest succeed, the errors for the +failures can be made available to the user, alongside the information about +the successes. + +#### Failure (irrelevant to the user) + +One or more *non-recoverable* errors can be returned at the _top level_. These +are things over which the **user** has little to no control, and should mainly +be system or programming problems, that a **developer** needs to know about. +In this case there is no `data`: + +```javascript +{ + errors: [ + {"message": "argument error: expected an integer, got null"}, + ] +} +``` + +This is the result of raising an error during the mutation. In our implementation, +the messages of argument errors and validation errors are returned to the client, and all other +`StandardError` instances are caught, logged and presented to the client with the message set to `"Internal server error"`. +See [`GraphqlController`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/controllers/graphql_controller.rb) for details. + +These represent programming errors, such as: + +- A GraphQL syntax error, where an `Int` was passed instead of a `String`, or a required argument was not present. +- Errors in our schema, such as being unable to provide a value for a non-nullable field. +- System errors: for example, a Git storage exception, or database unavailability. + +The user should not be able to cause such errors in regular usage. This category +of errors should be treated as internal, and not shown to the user in specific +detail. + +We need to inform the user when the mutation fails, but we do not need to +tell them why, since they cannot have caused it, and nothing they can do will +fix it, although we may offer to retry the mutation. + +#### Categorizing errors + +When we write mutations, we need to be conscious about which of +these two categories an error state falls into (and communicate about this with +frontend developers to verify our assumptions). This means distinguishing the +needs of the _user_ from the needs of the _client_. + +> _Never catch an error unless the user needs to know about it._ + +If the user does need to know about it, communicate with frontend developers +to make sure the error information we are passing back is useful. + +See also the [frontend GraphQL guide](../development/fe_guide/graphql.md#handling-errors). + ## Validating arguments For validations of single arguments, use the @@ -794,8 +919,8 @@ For validations of single arguments, use the as normal. Sometimes a mutation or resolver may accept a number of optional -arguments, but still want to validate that at least one of the optional -arguments were given. In this situation, consider using the `#ready?` +arguments, but we still want to validate that at least one of the optional +arguments is provided. In this situation, consider using the `#ready?` method within your mutation or resolver to provide the validation. The `#ready?` method will be called before any work is done within the `#resolve` method. diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index 8322189ebe3..2fa2af17b03 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -11,173 +11,17 @@ We're striving to [dogfood](https://about.gitlab.com/handbook/engineering/#dogfo GitLab [CI/CD features and best-practices](../ci/yaml/README.md) as much as possible. -## Stages +## Overview -The current stages are: - -- `sync`: This stage is used to synchronize changes from to - . -- `prepare`: This stage includes jobs that prepare artifacts that are needed by - jobs in subsequent stages. -- `build-images`: This stage includes jobs that prepare Docker images - that are needed by jobs in subsequent stages or downstream pipelines. -- `fixtures`: This stage includes jobs that prepare fixtures needed by frontend tests. -- `test`: This stage includes most of the tests, DB/migration jobs, and static analysis jobs. -- `post-test`: This stage includes jobs that build reports or gather data from - the `test` stage's jobs (e.g. coverage, Knapsack metadata etc.). -- `review-prepare`: This stage includes a job that build the CNG images that are - later used by the (Helm) Review App deployment (see - [Review Apps](testing_guide/review_apps.md) for details). -- `review`: This stage includes jobs that deploy the GitLab and Docs Review Apps. -- `qa`: This stage includes jobs that perform QA tasks against the Review App - that is deployed in the previous stage. -- `post-qa`: This stage includes jobs that build reports or gather data from - the `qa` stage's jobs (e.g. Review App performance report). -- `pages`: This stage includes a job that deploys the various reports as - GitLab Pages (e.g. , - , - ). - -## Default image - -The default image is defined in . - -It includes Ruby, Go, Git, Git LFS, Chrome, Node, Yarn, PostgreSQL, and Graphics Magick. - -The images used in our pipelines are configured in the -[`gitlab-org/gitlab-build-images`](https://gitlab.com/gitlab-org/gitlab-build-images) -project, which is push-mirrored to -for redundancy. - -The current version of the build images can be found in the -["Used by GitLab section"](https://gitlab.com/gitlab-org/gitlab-build-images/blob/master/.gitlab-ci.yml). - -## Default variables - -In addition to the [predefined variables](../ci/variables/predefined_variables.md), -each pipeline includes default variables defined in -. - -## Common job definitions - -Most of the jobs [extend from a few CI definitions](../ci/yaml/README.md#extends) -defined in [`.gitlab/ci/global.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/global.gitlab-ci.yml) -that are scoped to a single [configuration parameter](../ci/yaml/README.md#configuration-parameters). - -| Job definitions | Description | -|------------------|-------------| -| `.default-tags` | Ensures a job has the `gitlab-org` tag to ensure it's using our dedicated runners. | -| `.default-retry` | Allows a job to [retry](../ci/yaml/README.md#retry) upon `unknown_failure`, `api_failure`, `runner_system_failure`, `job_execution_timeout`, or `stuck_or_timeout_failure`. | -| `.default-before_script` | Allows a job to use a default `before_script` definition suitable for Ruby/Rails tasks that may need a database running (e.g. tests). | -| `.default-cache` | Allows a job to use a default `cache` definition suitable for Ruby/Rails and frontend tasks. | -| `.use-pg11` | Allows a job to use the `postgres:11.6` and `redis:alpine` services. | -| `.use-pg11-ee` | Same as `.use-pg11` but also use the `docker.elastic.co/elasticsearch/elasticsearch:6.4.2` services. | -| `.use-kaniko` | Allows a job to use the `kaniko` tool to build Docker images. | -| `.as-if-foss` | Simulate the FOSS project by setting the `FOSS_ONLY='1'` environment variable. | - -## `workflow:rules` - -We're using the [`workflow:rules` keyword](../ci/yaml/README.md#workflowrules) to -define default rules to determine whether or not a pipeline is created. - -These rules are defined in -and are as follows: - -1. If `$FORCE_GITLAB_CI` is set, create a pipeline. -1. For merge requests, create a pipeline. -1. For `master` branch, create a pipeline (this includes on schedules, pushes, merges, etc.). -1. For tags, create a pipeline. -1. If `$GITLAB_INTERNAL` isn't set, don't create a pipeline. -1. For stable, auto-deploy, and security branches, create a pipeline. -1. For any other cases (e.g. when pushing a branch with no MR for it), no pipeline is created. - -## `rules`, `if:` conditions and `changes:` patterns - -We're using the [`rules` keyword](../ci/yaml/README.md#rules) extensively. - -All `rules` definitions are defined in -, -then included in individual jobs via [`extends`](../ci/yaml/README.md#extends). - -The `rules` definitions are composed of `if:` conditions and `changes:` patterns, -which are also defined in - -and included in `rules` definitions via [YAML anchors](../ci/yaml/README.md#anchors) - -### `if:` conditions - -| `if:` conditions | Description | Notes | -|------------------|-------------|-------| -| `if-not-canonical-namespace` | Matches if the project isn't in the canonical (`gitlab-org/`) or security (`gitlab-org/security`) namespace. | Use to create a job for forks (by using `when: on_success\|manual`), or **not** create a job for forks (by using `when: never`). | -| `if-not-ee` | Matches if the project isn't EE (i.e. project name isn't `gitlab` or `gitlab-ee`). | Use to create a job only in the FOSS project (by using `when: on_success|manual`), or **not** create a job if the project is EE (by using `when: never`). | -| `if-not-foss` | Matches if the project isn't FOSS (i.e. project name isn't `gitlab-foss`, `gitlab-ce`, or `gitlabhq`). | Use to create a job only in the EE project (by using `when: on_success|manual`), or **not** create a job if the project is FOSS (by using `when: never`). | -| `if-default-refs` | Matches if the pipeline is for `master`, `/^[\d-]+-stable(-ee)?$/` (stable branches), `/^\d+-\d+-auto-deploy-\d+$/` (auto-deploy branches), `/^security\//` (security branches), merge requests, and tags. | Note that jobs won't be created for branches with this default configuration. | -| `if-master-refs` | Matches if the current branch is `master`. | | -| `if-master-or-tag` | Matches if the pipeline is for the `master` branch or for a tag. | | -| `if-merge-request` | Matches if the pipeline is for a merge request. | | -| `if-nightly-master-schedule` | Matches if the pipeline is for a `master` scheduled pipeline with `$NIGHTLY` set. | | -| `if-dot-com-gitlab-org-schedule` | Limits jobs creation to scheduled pipelines for the `gitlab-org` group on GitLab.com. | | -| `if-dot-com-gitlab-org-master` | Limits jobs creation to the `master` branch for the `gitlab-org` group on GitLab.com. | | -| `if-dot-com-gitlab-org-merge-request` | Limits jobs creation to merge requests for the `gitlab-org` group on GitLab.com. | | -| `if-dot-com-gitlab-org-and-security-tag` | Limits job creation to tags for the `gitlab-org` and `gitlab-org/security` groups on GitLab.com. | | -| `if-dot-com-gitlab-org-and-security-merge-request` | Limit jobs creation to merge requests for the `gitlab-org` and `gitlab-org/security` groups on GitLab.com. | | -| `if-dot-com-ee-schedule` | Limits jobs to scheduled pipelines for the `gitlab-org/gitlab` project on GitLab.com. | | -| `if-cache-credentials-schedule` | Limits jobs to scheduled pipelines with the `$CI_REPO_CACHE_CREDENTIALS` variable set. | | - -### `changes:` patterns - -| `changes:` patterns | Description | -|------------------------------|--------------------------------------------------------------------------| -| `ci-patterns` | Only create job for CI config-related changes. | -| `yaml-patterns` | Only create job for YAML-related changes. | -| `docs-patterns` | Only create job for docs-related changes. | -| `frontend-dependency-patterns` | Only create job when frontend dependencies are updated (i.e. `package.json`, and `yarn.lock`). changes. | -| `frontend-patterns` | Only create job for frontend-related changes. | -| `backstage-patterns` | Only create job for backstage-related changes (i.e. Danger, fixtures, RuboCop, specs). | -| `code-patterns` | Only create job for code-related changes. | -| `qa-patterns` | Only create job for QA-related changes. | -| `code-backstage-patterns` | Combination of `code-patterns` and `backstage-patterns`. | -| `code-qa-patterns` | Combination of `code-patterns` and `qa-patterns`. | -| `code-backstage-qa-patterns` | Combination of `code-patterns`, `backstage-patterns`, and `qa-patterns`. | - -## Interruptible jobs pipelines - -By default, all jobs are [interruptible](../ci/yaml/README.md#interruptible), except the -`dont-interrupt-me` job which runs automatically on `master`, and is `manual` -otherwise. - -If you want a running pipeline to finish even if you push new commits to a merge -request, be sure to start the `dont-interrupt-me` job before pushing. - -## PostgreSQL versions testing - -### Current versions testing - -| Where? | PostgreSQL version | -| ------ | ------ | -| MRs | 11 | -| `master` (non-scheduled pipelines) | 11 | -| 2-hourly scheduled pipelines | 11 | - -### Long-term plan - -We follow the [PostgreSQL versions shipped with Omnibus GitLab](https://docs.gitlab.com/omnibus/package-information/postgresql_versions.html): - -| PostgreSQL version | 12.10 (April 2020) | 13.0 (May 2020) | 13.1 (June 2020) | 13.2 (July 2020) | 13.3 (August 2020) | 13.4, 13.5 | 13.6 (November 2020) | 14.0 (May 2021?) | -| ------ | ------------------ | --------------- | ---------------- | ---------------- | ------------------ | ------------ | -------------------- | ---------------- | -| PG9.6 | MRs/`master`/`2-hour`/`nightly` | - | - | - | - | - | - | - | -| PG10 | `nightly` | - | - | - | - | - | - | - | -| PG11 | `master`/`2-hour` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | `nightly` | - | -| PG12 | - | - | - | - | `master`/`2-hour` | `master`/`2-hour` | MRs/`master`/`2-hour`/`nightly` | `master`/`2-hour` | -| PG13 | - | - | - | - | - | - | - | MRs/`master`/`2-hour`/`nightly` | - -## Pipeline types +### Pipeline types Since we use the [`rules:`](../ci/yaml/README.md#rules) and [`needs:`](../ci/yaml/README.md#needs) keywords extensively, we have four main pipeline types which are described below. Note that an MR that includes multiple types of changes would have a pipelines that include jobs from multiple types (e.g. a combination of docs-only and code-only pipelines). -### Docs-only MR pipeline +#### Docs-only MR pipeline + +
Click to view the graph Reference pipeline: @@ -191,7 +35,11 @@ graph LR end ``` -### Code-only MR pipeline +
+ +#### Code-only MR pipeline + +
Click to view the graph Reference pipeline: @@ -204,11 +52,11 @@ graph RL; click 1-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8100542&udv=0" 1-2["build-qa-image (3.4 minutes)"]; click 1-2 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914325&udv=0" - 1-3["compile-assets pull-cache (9.06 minutes)"]; + 1-3["compile-test-assets (9.06 minutes)"]; click 1-3 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914317&udv=0" - 1-4["compile-assets pull-cache as-if-foss (8.35 minutes)"]; + 1-4["compile-test-assets as-if-foss (8.35 minutes)"]; click 1-4 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8356616&udv=0" - 1-5["gitlab:assets:compile pull-cache (22 minutes)"]; + 1-5["compile-production-assets (22 minutes)"]; click 1-5 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914312&udv=0" 1-6["setup-test-env (8.22 minutes)"]; click 1-6 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914315&udv=0" @@ -224,6 +72,8 @@ graph RL; 1-18["kubesec-sast"]; 1-19["nodejs-scan-sast"]; 1-20["secrets-sast"]; + 1-21["static-analysis (17 minutes)"]; + click 1-21 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914471&udv=0" class 1-3 criticalPath; class 1-6 criticalPath; @@ -241,8 +91,6 @@ graph RL; 2_1-1 & 2_1-2 & 2_1-3 & 2_1-4 --> 1-6; end - 2_2-1["static-analysis (17 minutes)"]; - click 2_2-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914471&udv=0" 2_2-2["frontend-fixtures (17.2 minutes)"]; class 2_2-2 criticalPath; click 2_2-2 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=7910143&udv=0" @@ -252,13 +100,13 @@ graph RL; click 2_2-4 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8356727&udv=0" 2_2-5["webpack-dev-server (6.1 minutes)"]; click 2_2-5 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8404303&udv=0" - subgraph "Needs `setup-test-env` & `compile-assets`"; - 2_2-1 & 2_2-2 & 2_2-4 & 2_2-5 --> 1-6 & 1-3; + subgraph "Needs `setup-test-env` & `compile-test-assets`"; + 2_2-2 & 2_2-4 & 2_2-5 --> 1-6 & 1-3; 2_2-3 --> 1-6 & 1-4; end 2_3-1["build-assets-image (2.5 minutes)"]; - subgraph "Needs `gitlab:assets:compile`"; + subgraph "Needs `compile-production-assets`"; 2_3-1 --> 1-5 end @@ -269,7 +117,7 @@ graph RL; end 2_5-1["rspec & db jobs (12-22 minutes)"]; - subgraph "Needs `compile-assets`, `setup-test-env`, & `retrieve-tests-metadata`"; + subgraph "Needs `compile-test-assets`, `setup-test-env`, & `retrieve-tests-metadata`"; 2_5-1 --> 1-3 & 1-6 & 1-14; class 2_5-1 criticalPath; click 2_5-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations" @@ -304,7 +152,11 @@ graph RL; end ``` -### Frontend-only MR pipeline +
+ +#### Frontend-only MR pipeline + +
Click to view the graph Reference pipeline: @@ -317,11 +169,11 @@ graph RL; click 1-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8100542&udv=0" 1-2["build-qa-image (3.4 minutes)"]; click 1-2 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914325&udv=0" - 1-3["compile-assets pull-cache (9.06 minutes)"]; + 1-3["compile-test-assets (9.06 minutes)"]; click 1-3 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914317&udv=0" - 1-4["compile-assets pull-cache as-if-foss (8.35 minutes)"]; + 1-4["compile-test-assets as-if-foss (8.35 minutes)"]; click 1-4 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8356616&udv=0" - 1-5["gitlab:assets:compile pull-cache (22 minutes)"]; + 1-5["compile-production-assets (22 minutes)"]; click 1-5 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914312&udv=0" 1-6["setup-test-env (8.22 minutes)"]; click 1-6 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914315&udv=0" @@ -337,6 +189,8 @@ graph RL; 1-18["kubesec-sast"]; 1-19["nodejs-scan-sast"]; 1-20["secrets-sast"]; + 1-21["static-analysis (17 minutes)"]; + click 1-21 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914471&udv=0" class 1-3 criticalPath; class 1-5 criticalPath; @@ -355,8 +209,6 @@ graph RL; 2_1-1 & 2_1-2 & 2_1-3 & 2_1-4 --> 1-6; end - 2_2-1["static-analysis (17 minutes)"]; - click 2_2-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914471&udv=0" 2_2-2["frontend-fixtures (17.2 minutes)"]; class 2_2-2 criticalPath; click 2_2-2 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=7910143&udv=0" @@ -366,14 +218,14 @@ graph RL; click 2_2-4 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8356727&udv=0" 2_2-5["webpack-dev-server (6.1 minutes)"]; click 2_2-5 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8404303&udv=0" - subgraph "Needs `setup-test-env` & `compile-assets`"; - 2_2-1 & 2_2-2 & 2_2-4 & 2_2-5 --> 1-6 & 1-3; + subgraph "Needs `setup-test-env` & `compile-test-assets`"; + 2_2-2 & 2_2-4 & 2_2-5 --> 1-6 & 1-3; 2_2-3 --> 1-6 & 1-4; end 2_3-1["build-assets-image (2.5 minutes)"]; class 2_3-1 criticalPath; - subgraph "Needs `gitlab:assets:compile`"; + subgraph "Needs `compile-production-assets`"; 2_3-1 --> 1-5 end @@ -384,7 +236,7 @@ graph RL; end 2_5-1["rspec & db jobs (12-22 minutes)"]; - subgraph "Needs `compile-assets`, `setup-test-env, & `retrieve-tests-metadata`"; + subgraph "Needs `compile-test-assets`, `setup-test-env, & `retrieve-tests-metadata`"; 2_5-1 --> 1-3 & 1-6 & 1-14; class 2_5-1 criticalPath; click 2_5-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations" @@ -444,7 +296,11 @@ graph RL; end ``` -### QA-only MR pipeline +
+ +#### QA-only MR pipeline + +
Click to view the graph Reference pipeline: @@ -457,11 +313,11 @@ graph RL; click 1-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8100542&udv=0" 1-2["build-qa-image (3.4 minutes)"]; click 1-2 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914325&udv=0" - 1-3["compile-assets pull-cache (9.06 minutes)"]; + 1-3["compile-test-assets (9.06 minutes)"]; click 1-3 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914317&udv=0" - 1-4["compile-assets pull-cache as-if-foss (8.35 minutes)"]; + 1-4["compile-test-assets as-if-foss (8.35 minutes)"]; click 1-4 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=8356616&udv=0" - 1-5["gitlab:assets:compile pull-cache (22 minutes)"]; + 1-5["compile-production-assets (22 minutes)"]; click 1-5 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914312&udv=0" 1-6["setup-test-env (8.22 minutes)"]; click 1-6 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914315&udv=0" @@ -477,6 +333,8 @@ graph RL; 1-18["kubesec-sast"]; 1-19["nodejs-scan-sast"]; 1-20["secrets-sast"]; + 1-21["static-analysis (17 minutes)"]; + click 1-21 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914471&udv=0" class 1-5 criticalPath; end @@ -487,14 +345,8 @@ graph RL; 2_1-1 --> 1-6; end - 2_2-1["static-analysis (17 minutes)"]; - click 2_2-1 "https://app.periscopedata.com/app/gitlab/652085/Engineering-Productivity---Pipeline-Build-Durations?widget=6914471&udv=0" - subgraph "Needs `setup-test-env` & `compile-assets`"; - 2_2-1 --> 1-6 & 1-3; - end - 2_3-1["build-assets-image (2.5 minutes)"]; - subgraph "Needs `gitlab:assets:compile`"; + subgraph "Needs `compile-production-assets`"; 2_3-1 --> 1-5 class 2_3-1 criticalPath; end @@ -507,16 +359,56 @@ graph RL; end ``` -## Test jobs +
+ +### `workflow:rules` + +We're using the [`workflow:rules` keyword](../ci/yaml/README.md#workflowrules) to +define default rules to determine whether or not a pipeline is created. + +These rules are defined in +and are as follows: + +1. If `$FORCE_GITLAB_CI` is set, create a pipeline. +1. For merge requests, create a pipeline. +1. For `master` branch, create a pipeline (this includes on schedules, pushes, merges, etc.). +1. For tags, create a pipeline. +1. If `$GITLAB_INTERNAL` isn't set, don't create a pipeline. +1. For stable, auto-deploy, and security branches, create a pipeline. +1. For any other cases (e.g. when pushing a branch with no MR for it), no pipeline is created. + +### PostgreSQL versions testing + +#### Current versions testing + +| Where? | PostgreSQL version | +| ------ | ------ | +| MRs | 11 | +| `master` (non-scheduled pipelines) | 11 | +| 2-hourly scheduled pipelines | 11 | + +#### Long-term plan + +We follow the [PostgreSQL versions shipped with Omnibus GitLab](https://docs.gitlab.com/omnibus/package-information/postgresql_versions.html): + +| PostgreSQL version | 12.10 (April 2020) | 13.0 (May 2020) | 13.1 (June 2020) | 13.2 (July 2020) | 13.3 (August 2020) | 13.4, 13.5 | 13.6 (November 2020) | 14.0 (May 2021?) | +| ------ | ------------------ | --------------- | ---------------- | ---------------- | ------------------ | ------------ | -------------------- | ---------------- | +| PG9.6 | MRs/`master`/`2-hour`/`nightly` | - | - | - | - | - | - | - | +| PG10 | `nightly` | - | - | - | - | - | - | - | +| PG11 | `master`/`2-hour` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | MRs/`master`/`2-hour`/`nightly` | `nightly` | - | +| PG12 | - | - | - | - | `master`/`2-hour` | `master`/`2-hour` | MRs/`master`/`2-hour`/`nightly` | `master`/`2-hour` | +| PG13 | - | - | - | - | - | - | - | MRs/`master`/`2-hour`/`nightly` | + +### Test jobs Consult [GitLab tests in the Continuous Integration (CI) context](testing_guide/ci.md) for more information. -## Review app jobs +### Review app jobs Consult the [Review Apps](testing_guide/review_apps.md) dedicated page for more information. -## As-if-FOSS jobs +### As-if-FOSS jobs The `* as-if-foss` jobs allows to run GitLab's test suite "as-if-FOSS", meaning as if the jobs would run in the context of the `gitlab-org/gitlab-foss` project. These jobs are only created in the following cases: @@ -532,7 +424,39 @@ folders removed before the tests start running. The intent is to ensure that a change won't introduce a failure once the `gitlab-org/gitlab` project will be synced to the `gitlab-org/gitlab-foss` project. -## Pre-clone step +## Performance + +### Interruptible pipelines + +By default, all jobs are [interruptible](../ci/yaml/README.md#interruptible), except the +`dont-interrupt-me` job which runs automatically on `master`, and is `manual` +otherwise. + +If you want a running pipeline to finish even if you push new commits to a merge +request, be sure to start the `dont-interrupt-me` job before pushing. + +### Caching strategy + +1. All jobs must only pull caches by default. +1. All jobs must be able to pass with an empty cache. In other words, caches are only there to speed up jobs. +1. We currently have 6 different caches defined in + [`.gitlab/ci/global.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/global.gitlab-ci.yml), + with fixed keys: + - `.rails-cache`. + - `.static-analysis-cache`. + - `.qa-cache` + - `.yarn-cache`. + - `.assets-compile-cache` (the key includes `${NODE_ENV}` so it's actually two different caches). +1. Only 6 specific jobs, running in 2-hourly scheduled pipelines, are pushing (i.e. updating) to the caches: + - `update-rails-cache`, defined in [`.gitlab/ci/rails.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/rails.gitlab-ci.yml). + - `update-static-analysis-cache`, defined in [`.gitlab/ci/rails.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/rails.gitlab-ci.yml). + - `update-qa-cache`, defined in [`.gitlab/ci/qa.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/qa.gitlab-ci.yml). + - `update-assets-compile-production-cache`, defined in [`.gitlab/ci/frontend.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/frontend.gitlab-ci.yml). + - `update-assets-compile-test-cache`, defined in [`.gitlab/ci/frontend.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/frontend.gitlab-ci.yml). + - `update-yarn-cache`, defined in [`.gitlab/ci/frontend.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/frontend.gitlab-ci.yml). +1. These jobs will run in merge requests whose title include `UPDATE CACHE`. + +### Pre-clone step The `gitlab-org/gitlab` project on GitLab.com uses a [pre-clone step](https://gitlab.com/gitlab-org/gitlab/-/issues/39134) to seed the project with a recent archive of the repository. This is done for @@ -597,6 +521,124 @@ credentials are stored in the 1Password GitLab.com Production vault. Note that this bucket should be located in the same continent as the runner, or [network egress charges will apply](https://cloud.google.com/storage/pricing). +## CI configuration internals + +### Stages + +The current stages are: + +- `sync`: This stage is used to synchronize changes from to + . +- `prepare`: This stage includes jobs that prepare artifacts that are needed by + jobs in subsequent stages. +- `build-images`: This stage includes jobs that prepare Docker images + that are needed by jobs in subsequent stages or downstream pipelines. +- `fixtures`: This stage includes jobs that prepare fixtures needed by frontend tests. +- `test`: This stage includes most of the tests, DB/migration jobs, and static analysis jobs. +- `post-test`: This stage includes jobs that build reports or gather data from + the `test` stage's jobs (e.g. coverage, Knapsack metadata etc.). +- `review-prepare`: This stage includes a job that build the CNG images that are + later used by the (Helm) Review App deployment (see + [Review Apps](testing_guide/review_apps.md) for details). +- `review`: This stage includes jobs that deploy the GitLab and Docs Review Apps. +- `qa`: This stage includes jobs that perform QA tasks against the Review App + that is deployed in the previous stage. +- `post-qa`: This stage includes jobs that build reports or gather data from + the `qa` stage's jobs (e.g. Review App performance report). +- `pages`: This stage includes a job that deploys the various reports as + GitLab Pages (e.g. , + , + ). + +### Default image + +The default image is defined in . + +It includes Ruby, Go, Git, Git LFS, Chrome, Node, Yarn, PostgreSQL, and Graphics Magick. + +The images used in our pipelines are configured in the +[`gitlab-org/gitlab-build-images`](https://gitlab.com/gitlab-org/gitlab-build-images) +project, which is push-mirrored to +for redundancy. + +The current version of the build images can be found in the +["Used by GitLab section"](https://gitlab.com/gitlab-org/gitlab-build-images/blob/master/.gitlab-ci.yml). + +### Default variables + +In addition to the [predefined variables](../ci/variables/predefined_variables.md), +each pipeline includes default variables defined in +. + +### Common job definitions + +Most of the jobs [extend from a few CI definitions](../ci/yaml/README.md#extends) +defined in [`.gitlab/ci/global.gitlab-ci.yml`](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/ci/global.gitlab-ci.yml) +that are scoped to a single [configuration parameter](../ci/yaml/README.md#configuration-parameters). + +| Job definitions | Description | +|------------------|-------------| +| `.default-tags` | Ensures a job has the `gitlab-org` tag to ensure it's using our dedicated runners. | +| `.default-retry` | Allows a job to [retry](../ci/yaml/README.md#retry) upon `unknown_failure`, `api_failure`, `runner_system_failure`, `job_execution_timeout`, or `stuck_or_timeout_failure`. | +| `.default-before_script` | Allows a job to use a default `before_script` definition suitable for Ruby/Rails tasks that may need a database running (e.g. tests). | +| `.rails-cache` | Allows a job to use a default `cache` definition suitable for Ruby/Rails tasks. | +| `.static-analysis-cache` | Allows a job to use a default `cache` definition suitable for static analysis tasks. | +| `.yarn-cache` | Allows a job to use a default `cache` definition suitable for frontend jobs that do a `yarn install`. | +| `.assets-compile-cache` | Allows a job to use a default `cache` definition suitable for frontend jobs that compile assets. | +| `.use-pg11` | Allows a job to use the `postgres:11.6` and `redis:alpine` services. | +| `.use-pg11-ee` | Same as `.use-pg11` but also use the `docker.elastic.co/elasticsearch/elasticsearch:6.4.2` services. | +| `.use-kaniko` | Allows a job to use the `kaniko` tool to build Docker images. | +| `.as-if-foss` | Simulate the FOSS project by setting the `FOSS_ONLY='1'` environment variable. | + +### `rules`, `if:` conditions and `changes:` patterns + +We're using the [`rules` keyword](../ci/yaml/README.md#rules) extensively. + +All `rules` definitions are defined in +, +then included in individual jobs via [`extends`](../ci/yaml/README.md#extends). + +The `rules` definitions are composed of `if:` conditions and `changes:` patterns, +which are also defined in + +and included in `rules` definitions via [YAML anchors](../ci/yaml/README.md#anchors) + +#### `if:` conditions + +| `if:` conditions | Description | Notes | +|------------------|-------------|-------| +| `if-not-canonical-namespace` | Matches if the project isn't in the canonical (`gitlab-org/`) or security (`gitlab-org/security`) namespace. | Use to create a job for forks (by using `when: on_success\|manual`), or **not** create a job for forks (by using `when: never`). | +| `if-not-ee` | Matches if the project isn't EE (i.e. project name isn't `gitlab` or `gitlab-ee`). | Use to create a job only in the FOSS project (by using `when: on_success|manual`), or **not** create a job if the project is EE (by using `when: never`). | +| `if-not-foss` | Matches if the project isn't FOSS (i.e. project name isn't `gitlab-foss`, `gitlab-ce`, or `gitlabhq`). | Use to create a job only in the EE project (by using `when: on_success|manual`), or **not** create a job if the project is FOSS (by using `when: never`). | +| `if-default-refs` | Matches if the pipeline is for `master`, `/^[\d-]+-stable(-ee)?$/` (stable branches), `/^\d+-\d+-auto-deploy-\d+$/` (auto-deploy branches), `/^security\//` (security branches), merge requests, and tags. | Note that jobs won't be created for branches with this default configuration. | +| `if-master-refs` | Matches if the current branch is `master`. | | +| `if-master-or-tag` | Matches if the pipeline is for the `master` branch or for a tag. | | +| `if-merge-request` | Matches if the pipeline is for a merge request. | | +| `if-nightly-master-schedule` | Matches if the pipeline is for a `master` scheduled pipeline with `$NIGHTLY` set. | | +| `if-dot-com-gitlab-org-schedule` | Limits jobs creation to scheduled pipelines for the `gitlab-org` group on GitLab.com. | | +| `if-dot-com-gitlab-org-master` | Limits jobs creation to the `master` branch for the `gitlab-org` group on GitLab.com. | | +| `if-dot-com-gitlab-org-merge-request` | Limits jobs creation to merge requests for the `gitlab-org` group on GitLab.com. | | +| `if-dot-com-gitlab-org-and-security-tag` | Limits job creation to tags for the `gitlab-org` and `gitlab-org/security` groups on GitLab.com. | | +| `if-dot-com-gitlab-org-and-security-merge-request` | Limit jobs creation to merge requests for the `gitlab-org` and `gitlab-org/security` groups on GitLab.com. | | +| `if-dot-com-ee-schedule` | Limits jobs to scheduled pipelines for the `gitlab-org/gitlab` project on GitLab.com. | | +| `if-cache-credentials-schedule` | Limits jobs to scheduled pipelines with the `$CI_REPO_CACHE_CREDENTIALS` variable set. | | + +#### `changes:` patterns + +| `changes:` patterns | Description | +|------------------------------|--------------------------------------------------------------------------| +| `ci-patterns` | Only create job for CI config-related changes. | +| `yaml-patterns` | Only create job for YAML-related changes. | +| `docs-patterns` | Only create job for docs-related changes. | +| `frontend-dependency-patterns` | Only create job when frontend dependencies are updated (i.e. `package.json`, and `yarn.lock`). changes. | +| `frontend-patterns` | Only create job for frontend-related changes. | +| `backstage-patterns` | Only create job for backstage-related changes (i.e. Danger, fixtures, RuboCop, specs). | +| `code-patterns` | Only create job for code-related changes. | +| `qa-patterns` | Only create job for QA-related changes. | +| `code-backstage-patterns` | Combination of `code-patterns` and `backstage-patterns`. | +| `code-qa-patterns` | Combination of `code-patterns` and `qa-patterns`. | +| `code-backstage-qa-patterns` | Combination of `code-patterns`, `backstage-patterns`, and `qa-patterns`. | + --- [Return to Development documentation](README.md) diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 6d1951fa247..f85b2467360 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -357,7 +357,7 @@ Feature.enabled?(:my_feature2) # => false Feature.enabled?(:my_feature2, project1) # => true ``` -#### `stub_feature_flags` vs `Feature.get/enable` +#### `stub_feature_flags` vs `Feature.enable*` It is preferred to use `stub_feature_flags` for enabling feature flags in testing environment. This method provides a simple and well described @@ -378,10 +378,10 @@ stub_feature_flags(my_feature: [project, project2]) Feature.enable(:my_feature_2) # Good: enable my_feature for 50% of time -Feature.get(:my_feature_3).enable_percentage_of_time(50) +Feature.enable_percentage_of_time(:my_feature_3, 50) # Good: enable my_feature for 50% of actors/gates/things -Feature.get(:my_feature_4).enable_percentage_of_actors(50) +Feature.enable_percentage_of_actors(:my_feature_4, 50) ``` Each feature flag that has a defined state will be persisted diff --git a/doc/topics/autodevops/customize.md b/doc/topics/autodevops/customize.md index 98b858ed0a6..c7e0aa13adb 100644 --- a/doc/topics/autodevops/customize.md +++ b/doc/topics/autodevops/customize.md @@ -598,7 +598,7 @@ The banner can be disabled for: - By an administrator running the following in a Rails console: ```ruby - Feature.get(:auto_devops_banner_disabled).enable + Feature.enable(:auto_devops_banner_disabled) ``` - Through the REST API with an admin access token: diff --git a/doc/topics/autodevops/index.md b/doc/topics/autodevops/index.md index 351619e59ca..ecef65b28ea 100644 --- a/doc/topics/autodevops/index.md +++ b/doc/topics/autodevops/index.md @@ -346,7 +346,7 @@ of projects. From the console, enter the following command, replacing `10` with your desired percentage: ```ruby -Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(10) +Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 10) ``` ### Deployment strategy diff --git a/lib/api/features.rb b/lib/api/features.rb index f507919b055..3fb3fc92e42 100644 --- a/lib/api/features.rb +++ b/lib/api/features.rb @@ -61,7 +61,7 @@ module API mutually_exclusive :key, :project end post ':name' do - feature = Feature.get(params[:name]) + feature = Feature.get(params[:name]) # rubocop:disable Gitlab/AvoidFeatureGet targets = gate_targets(params) value = gate_value(params) key = gate_key(params) @@ -92,7 +92,7 @@ module API desc 'Remove the gate value for the given feature' delete ':name' do - Feature.get(params[:name]).remove + Feature.remove(params[:name]) no_content! end diff --git a/lib/feature.rb b/lib/feature.rb index c58e8215724..d995e0a988f 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -79,7 +79,7 @@ class Feature # for the feature yet and return the default. return default_enabled unless Gitlab::Database.exists? - feature = Feature.get(key) + feature = get(key) # If we're not default enabling the flag or the feature has been set, always evaluate. # `persisted?` can potentially generate DB queries and also checks for inclusion @@ -109,12 +109,41 @@ class Feature get(key).disable_group(group) end + def enable_percentage_of_time(key, percentage) + get(key).enable_percentage_of_time(percentage) + end + + def disable_percentage_of_time(key) + get(key).disable_percentage_of_time + end + + def enable_percentage_of_actors(key, percentage) + get(key).enable_percentage_of_actors(percentage) + end + + def disable_percentage_of_actors(key) + get(key).disable_percentage_of_actors + end + def remove(key) return unless persisted_name?(key) get(key).remove end + def reset + Gitlab::SafeRequestStore.delete(:flipper) if Gitlab::SafeRequestStore.active? + @flipper = nil + end + + # This method is called from config/initializers/flipper.rb and can be used + # to register Flipper groups. + # See https://docs.gitlab.com/ee/development/feature_flags.html#feature-groups + def register_feature_groups + end + + private + def flipper if Gitlab::SafeRequestStore.active? Gitlab::SafeRequestStore[:flipper] ||= build_flipper_instance @@ -123,11 +152,6 @@ class Feature end end - def reset - Gitlab::SafeRequestStore.delete(:flipper) if Gitlab::SafeRequestStore.active? - @flipper = nil - end - def build_flipper_instance active_record_adapter = Flipper::Adapters::ActiveRecord.new( feature_class: FlipperFeature, @@ -158,12 +182,6 @@ class Feature Gitlab.dev_or_test_env? end - # This method is called from config/initializers/flipper.rb and can be used - # to register Flipper groups. - # See https://docs.gitlab.com/ee/development/feature_flags.html#feature-groups - def register_feature_groups - end - def l1_cache_backend Gitlab::ProcessMemoryCache.cache_backend end diff --git a/lib/gitlab/ci/pipeline/chain/command.rb b/lib/gitlab/ci/pipeline/chain/command.rb index 73187401903..8118e7b2487 100644 --- a/lib/gitlab/ci/pipeline/chain/command.rb +++ b/lib/gitlab/ci/pipeline/chain/command.rb @@ -77,19 +77,18 @@ module Gitlab bridge&.parent_pipeline end - def duration_histogram - strong_memoize(:duration_histogram) do - name = :gitlab_ci_pipeline_creation_duration_seconds - comment = 'Pipeline creation duration' - labels = {} - buckets = [0.01, 0.05, 0.1, 0.5, 1.0, 2.0, 5.0, 20.0, 50.0, 240.0] - - Gitlab::Metrics.histogram(name, comment, labels, buckets) - end + def metrics + @metrics ||= Chain::Metrics.new end def observe_creation_duration(duration) - duration_histogram.observe({}, duration.seconds) + metrics.pipeline_creation_duration_histogram + .observe({}, duration.seconds) + end + + def observe_pipeline_size(pipeline) + metrics.pipeline_size_histogram + .observe({ source: pipeline.source.to_s }, pipeline.total_size) end end end diff --git a/lib/gitlab/ci/pipeline/chain/metrics.rb b/lib/gitlab/ci/pipeline/chain/metrics.rb new file mode 100644 index 00000000000..980ab2de9b0 --- /dev/null +++ b/lib/gitlab/ci/pipeline/chain/metrics.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Pipeline + module Chain + class Metrics + include Gitlab::Utils::StrongMemoize + + def pipeline_creation_duration_histogram + strong_memoize(:pipeline_creation_duration_histogram) do + name = :gitlab_ci_pipeline_creation_duration_seconds + comment = 'Pipeline creation duration' + labels = {} + buckets = [0.01, 0.05, 0.1, 0.5, 1.0, 2.0, 5.0, 20.0, 50.0, 240.0] + + ::Gitlab::Metrics.histogram(name, comment, labels, buckets) + end + end + + def pipeline_size_histogram + strong_memoize(:pipeline_size_histogram) do + name = :gitlab_ci_pipeline_size_builds + comment = 'Pipeline size' + labels = { source: nil } + buckets = [0, 1, 5, 10, 20, 50, 100, 200, 500, 1000] + + ::Gitlab::Metrics.histogram(name, comment, labels, buckets) + end + end + end + end + end + end +end diff --git a/lib/gitlab/ci/pipeline/chain/sequence.rb b/lib/gitlab/ci/pipeline/chain/sequence.rb index a7c671e76d3..204c7725214 100644 --- a/lib/gitlab/ci/pipeline/chain/sequence.rb +++ b/lib/gitlab/ci/pipeline/chain/sequence.rb @@ -27,6 +27,7 @@ module Gitlab yield @pipeline, self if block_given? @command.observe_creation_duration(Time.now - @start) + @command.observe_pipeline_size(@pipeline) end end diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb index 26209ea6d0c..62a88db7629 100644 --- a/lib/gitlab/experimentation.rb +++ b/lib/gitlab/experimentation.rb @@ -12,19 +12,21 @@ # # To enable the experiment for 10% of the users: # -# chatops: `/chatops run feature set experiment_key_experiment_percentage 10` -# console: `Feature.get(:experiment_key_experiment_percentage).enable_percentage_of_time(10)` +# chatops: `/chatops run feature set experiment_key_experiment_percentage 10 --actors` +# console: `Feature.enable_percentage_of_actors(:experiment_key_experiment_percentage, 10)` # # To disable the experiment: # # chatops: `/chatops run feature delete experiment_key_experiment_percentage` -# console: `Feature.get(:experiment_key_experiment_percentage).remove` +# console: `Feature.remove(:experiment_key_experiment_percentage)` # # To check the current rollout percentage: # # chatops: `/chatops run feature get experiment_key_experiment_percentage` # console: `Feature.get(:experiment_key_experiment_percentage).percentage_of_time_value` # + +# TODO: rewrite that module Gitlab module Experimentation EXPERIMENTS = { @@ -178,7 +180,7 @@ module Gitlab # When a feature does not exist, the `percentage_of_time_value` method will return 0 def experiment_percentage - @experiment_percentage ||= Feature.get(:"#{key}_experiment_percentage").percentage_of_time_value + @experiment_percentage ||= Feature.get(:"#{key}_experiment_percentage").percentage_of_time_value # rubocop:disable Gitlab/AvoidFeatureGet end end end diff --git a/lib/gitlab/jira_import.rb b/lib/gitlab/jira_import.rb index 3f56094956a..f7373a65f88 100644 --- a/lib/gitlab/jira_import.rb +++ b/lib/gitlab/jira_import.rb @@ -10,6 +10,18 @@ module Gitlab ITEMS_MAPPER_CACHE_KEY = 'jira-import/items-mapper/%{project_id}/%{collection_type}/%{jira_isssue_id}' ALREADY_IMPORTED_ITEMS_CACHE_KEY = 'jira-importer/already-imported/%{project}/%{collection_type}' + def self.validate_project_settings!(project, user: nil) + if user + raise Projects::ImportService::Error, _('Cannot import because issues are not available in this project.') unless project.feature_available?(:issues, user) + raise Projects::ImportService::Error, _('You do not have permissions to run the import.') unless user.can?(:admin_project, project) + end + + jira_service = project.jira_service + + raise Projects::ImportService::Error, _('Jira integration not configured.') unless jira_service&.active? + raise Projects::ImportService::Error, _('Unable to connect to the Jira instance. Please check your Jira integration configuration.') unless jira_service.test(nil)[:success] + end + def self.jira_issue_cache_key(project_id, jira_issue_id) ITEMS_MAPPER_CACHE_KEY % { project_id: project_id, collection_type: :issues, jira_isssue_id: jira_issue_id } end diff --git a/lib/gitlab/jira_import/base_importer.rb b/lib/gitlab/jira_import/base_importer.rb index 306736df30f..688254bf91f 100644 --- a/lib/gitlab/jira_import/base_importer.rb +++ b/lib/gitlab/jira_import/base_importer.rb @@ -6,7 +6,7 @@ module Gitlab attr_reader :project, :client, :formatter, :jira_project_key, :running_import def initialize(project) - project.validate_jira_import_settings! + Gitlab::JiraImport.validate_project_settings!(project) @running_import = project.latest_jira_import @jira_project_key = running_import&.jira_project_key diff --git a/lib/gitlab/metrics/methods.rb b/lib/gitlab/metrics/methods.rb index cee601ff14c..5955987541c 100644 --- a/lib/gitlab/metrics/methods.rb +++ b/lib/gitlab/metrics/methods.rb @@ -52,7 +52,7 @@ module Gitlab end def disabled_by_feature(options) - options.with_feature && !::Feature.get(options.with_feature).enabled? + options.with_feature && !::Feature.enabled?(options.with_feature) end def build_metric!(type, name, options) diff --git a/lib/gitlab/sourcegraph.rb b/lib/gitlab/sourcegraph.rb index d0f12c8364a..ec404ebd309 100644 --- a/lib/gitlab/sourcegraph.rb +++ b/lib/gitlab/sourcegraph.rb @@ -19,7 +19,7 @@ module Gitlab private def feature - Feature.get(:sourcegraph) + Feature.get(:sourcegraph) # rubocop:disable Gitlab/AvoidFeatureGet end end end diff --git a/lib/tasks/gitlab/features.rake b/lib/tasks/gitlab/features.rake index 9cf568c07fe..2309aa5d214 100644 --- a/lib/tasks/gitlab/features.rake +++ b/lib/tasks/gitlab/features.rake @@ -21,7 +21,7 @@ namespace :gitlab do Gitlab::Git::RuggedImpl::Repository::FEATURE_FLAGS.each do |flag| case status when nil - Feature.get(flag).remove + Feature.remove(flag) when true Feature.enable(flag) when false diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 60e6b5bcc53..6f55945829e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15411,6 +15411,9 @@ msgstr "" msgid "PackageRegistry|There was a problem fetching the details for this package." msgstr "" +msgid "PackageRegistry|This NuGet package has no dependencies." +msgstr "" + msgid "PackageRegistry|To widen your search, change or remove the filters above." msgstr "" @@ -25810,6 +25813,9 @@ msgstr "" msgid "branch name" msgstr "" +msgid "build pipeline reference mismatch" +msgstr "" + msgid "by" msgstr "" diff --git a/package.json b/package.json index 76a45c3bda2..fc972204f32 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "@babel/plugin-syntax-import-meta": "^7.8.3", "@babel/preset-env": "^7.8.4", "@gitlab/at.js": "1.5.5", - "@gitlab/svgs": "1.131.0", + "@gitlab/svgs": "1.133.0", "@gitlab/ui": "16.1.0", "@gitlab/visual-review-tools": "1.6.1", "@rails/actioncable": "^6.0.3", @@ -101,7 +101,7 @@ "katex": "^0.10.0", "lodash": "^4.17.15", "marked": "^0.3.12", - "mermaid": "^8.4.8", + "mermaid": "^8.5.1", "mitt": "^1.2.0", "monaco-editor": "^0.18.1", "monaco-editor-webpack-plugin": "^1.7.0", diff --git a/rubocop/cop/gitlab/avoid_feature_get.rb b/rubocop/cop/gitlab/avoid_feature_get.rb new file mode 100644 index 00000000000..e36e0b020c0 --- /dev/null +++ b/rubocop/cop/gitlab/avoid_feature_get.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that blacklists the use of `Feature.get`. + class AvoidFeatureGet < RuboCop::Cop::Cop + MSG = 'Use `Feature.enable/disable` methods instead of `Feature.get`. ' \ + 'See doc/development/testing_guide/best_practices.md#feature-flags-in-tests for more information.' + + def_node_matcher :feature_get?, <<~PATTERN + (send (const nil? :Feature) :get ...) + PATTERN + + def_node_matcher :global_feature_get?, <<~PATTERN + (send (const (cbase) :Feature) :get ...) + PATTERN + + def on_send(node) + return unless feature_get?(node) || global_feature_get?(node) + + add_offense(node, location: :selector) + end + end + end + end +end diff --git a/spec/controllers/admin/integrations_controller_spec.rb b/spec/controllers/admin/integrations_controller_spec.rb index 94d38353189..cb8282a6c4a 100644 --- a/spec/controllers/admin/integrations_controller_spec.rb +++ b/spec/controllers/admin/integrations_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe Admin::IntegrationsController do let(:admin) { create(:admin) } + let(:integration) { create(:jira_service, :instance) } before do sign_in(admin) @@ -33,8 +34,6 @@ describe Admin::IntegrationsController do end describe '#update' do - let(:integration) { create(:jira_service, :instance) } - before do allow(PropagateIntegrationWorker).to receive(:perform_async) @@ -68,4 +67,14 @@ describe Admin::IntegrationsController do end end end + + describe '#custom_integration_projects' do + it 'calls to get the custom integration projects' do + allow(Project).to receive_message_chain(:with_custom_integration_compared_to, :page, :per) + + get :custom_integration_projects, params: { id: integration.class.to_param } + + expect(Project).to have_received(:with_custom_integration_compared_to).with(integration) + end + end end diff --git a/spec/controllers/concerns/sourcegraph_decorator_spec.rb b/spec/controllers/concerns/sourcegraph_decorator_spec.rb index 87dadc6faa4..fc0ad0a70f9 100644 --- a/spec/controllers/concerns/sourcegraph_decorator_spec.rb +++ b/spec/controllers/concerns/sourcegraph_decorator_spec.rb @@ -36,10 +36,6 @@ describe SourcegraphDecorator do sign_in user if user end - after do - Feature.get(:sourcegraph).disable - end - subject do get :index, format: format diff --git a/spec/features/merge_request/user_clicks_merge_request_tabs_spec.rb b/spec/features/merge_request/user_clicks_merge_request_tabs_spec.rb new file mode 100644 index 00000000000..8295a1770bd --- /dev/null +++ b/spec/features/merge_request/user_clicks_merge_request_tabs_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'User clicks on merge request tabs', :js do + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + it 'adds entry to page history' do + visit('/') + visit(merge_request_path(merge_request)) + click_link('Changes') + + expect(current_url).to match(/diffs$/) + + page.driver.go_back + + expect(current_url).to match(merge_request_path(merge_request)) + + page.driver.go_back + + expect(current_url).to match('/') + end +end diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 86db6b3572f..879698ffff3 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -51,6 +51,7 @@ import axios from '~/lib/utils/axios_utils'; import testAction from '../../helpers/vuex_action_helper'; import * as utils from '~/diffs/store/utils'; import * as commonUtils from '~/lib/utils/common_utils'; +import { mergeUrlParams } from '~/lib/utils/url_utility'; import { useLocalStorageSpy } from 'helpers/local_storage_helper'; import createFlash from '~/flash'; @@ -173,19 +174,44 @@ describe('DiffsStoreActions', () => { }); describe('fetchDiffFilesBatch', () => { + let mock; + + beforeEach(() => { + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.restore(); + }); + it('should fetch batch diff files', done => { const endpointBatch = '/fetch/diffs_batch'; - const mock = new MockAdapter(axios); const res1 = { diff_files: [], pagination: { next_page: 2 } }; const res2 = { diff_files: [], pagination: {} }; mock - .onGet(endpointBatch, { - params: { page: 1, per_page: DIFFS_PER_PAGE, w: '1', view: 'inline' }, - }) + .onGet( + mergeUrlParams( + { + per_page: DIFFS_PER_PAGE, + w: '1', + view: 'inline', + page: 1, + }, + endpointBatch, + ), + ) .reply(200, res1) - .onGet(endpointBatch, { - params: { page: 2, per_page: DIFFS_PER_PAGE, w: '1', view: 'inline' }, - }) + .onGet( + mergeUrlParams( + { + per_page: DIFFS_PER_PAGE, + w: '1', + view: 'inline', + page: 2, + }, + endpointBatch, + ), + ) .reply(200, res2); testAction( @@ -202,12 +228,34 @@ describe('DiffsStoreActions', () => { { type: types.SET_RETRIEVING_BATCHES, payload: false }, ], [], - () => { - mock.restore(); - done(); - }, + done, ); }); + + it.each` + viewStyle | otherView + ${'inline'} | ${'parallel'} + ${'parallel'} | ${'inline'} + `( + 'should make a request with the view parameter "$viewStyle" when the batchEndpoint already contains "$otherView"', + ({ viewStyle, otherView }) => { + const endpointBatch = '/fetch/diffs_batch'; + + fetchDiffFilesBatch({ + commit: () => {}, + state: { + endpointBatch: `${endpointBatch}?view=${otherView}`, + useSingleDiffStyle: true, + diffViewType: viewStyle, + }, + }) + .then(() => { + expect(mock.history.get[0].url).toContain(`view=${viewStyle}`); + expect(mock.history.get[0].url).not.toContain(`view=${otherView}`); + }) + .catch(() => {}); + }, + ); }); describe('fetchDiffFilesMeta', () => { @@ -278,15 +326,24 @@ describe('DiffsStoreActions', () => { }); describe('fetchDiffFilesBatch', () => { + let mock; + + beforeEach(() => { + mock = new MockAdapter(axios); + }); + + afterEach(() => { + mock.restore(); + }); + it('should fetch batch diff files', done => { const endpointBatch = '/fetch/diffs_batch'; - const mock = new MockAdapter(axios); const res1 = { diff_files: [], pagination: { next_page: 2 } }; const res2 = { diff_files: [], pagination: {} }; mock - .onGet(endpointBatch, { params: { page: 1, per_page: DIFFS_PER_PAGE, w: '1' } }) + .onGet(mergeUrlParams({ per_page: DIFFS_PER_PAGE, w: '1', page: 1 }, endpointBatch)) .reply(200, res1) - .onGet(endpointBatch, { params: { page: 2, per_page: DIFFS_PER_PAGE, w: '1' } }) + .onGet(mergeUrlParams({ per_page: DIFFS_PER_PAGE, w: '1', page: 2 }, endpointBatch)) .reply(200, res2); testAction( @@ -303,12 +360,33 @@ describe('DiffsStoreActions', () => { { type: types.SET_RETRIEVING_BATCHES, payload: false }, ], [], - () => { - mock.restore(); - done(); - }, + done, ); }); + + it.each` + querystrings | requestUrl + ${'?view=parallel'} | ${'/fetch/diffs_batch?view=parallel'} + ${'?view=inline'} | ${'/fetch/diffs_batch?view=inline'} + ${''} | ${'/fetch/diffs_batch'} + `( + 'should use the endpoint $requestUrl if the endpointBatch in state includes `$querystrings` as a querystring', + ({ querystrings, requestUrl }) => { + const endpointBatch = '/fetch/diffs_batch'; + + fetchDiffFilesBatch({ + commit: () => {}, + state: { + endpointBatch: `${endpointBatch}${querystrings}`, + diffViewType: 'inline', + }, + }) + .then(() => { + expect(mock.history.get[0].url).toEqual(requestUrl); + }) + .catch(() => {}); + }, + ); }); describe('fetchDiffFilesMeta', () => { diff --git a/spec/helpers/auto_devops_helper_spec.rb b/spec/helpers/auto_devops_helper_spec.rb index d06548f1595..e0fecb0c159 100644 --- a/spec/helpers/auto_devops_helper_spec.rb +++ b/spec/helpers/auto_devops_helper_spec.rb @@ -13,7 +13,7 @@ describe AutoDevopsHelper do allow(helper).to receive(:can?).with(user, :admin_pipeline, project) { allowed } allow(helper).to receive(:current_user) { user } - Feature.get(:auto_devops_banner_disabled).disable + stub_feature_flags(auto_devops_banner_disabled: false) end subject { helper.show_auto_devops_callout?(project) } @@ -32,7 +32,7 @@ describe AutoDevopsHelper do context 'when the banner is disabled by feature flag' do before do - Feature.get(:auto_devops_banner_disabled).enable + stub_feature_flags(auto_devops_banner_disabled: true) end it { is_expected.to be_falsy } diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index cbb61333d77..2b5fde9532d 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -30,7 +30,7 @@ describe('MergeRequestTabs', function() { setLocation(); this.spies = { - history: spyOn(window.history, 'replaceState').and.callFake(function() {}), + history: spyOn(window.history, 'pushState').and.callFake(function() {}), }; }); @@ -142,6 +142,7 @@ describe('MergeRequestTabs', function() { afterEach(() => { mock.restore(); + window.history.replaceState({}, '', '/'); }); it('changes from commits', function() { @@ -194,11 +195,21 @@ describe('MergeRequestTabs', function() { setLocation({ pathname: '/foo/bar/-/merge_requests/1', }); + window.history.replaceState( + { + url: window.location.href, + action: 'show', + }, + document.title, + window.location.href, + ); + const newState = this.subject('commits'); expect(this.spies.history).toHaveBeenCalledWith( { url: newState, + action: 'commits', }, document.title, newState, diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 9f82b663daf..37f8d3ad47d 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -142,7 +142,7 @@ describe Feature, stub_feature_flags: false do expect(Flipper).to receive(:new).once.and_call_original 2.times do - described_class.flipper + described_class.send(:flipper) end end end @@ -151,9 +151,9 @@ describe Feature, stub_feature_flags: false do it 'memoizes the Flipper instance' do expect(Flipper).to receive(:new).once.and_call_original - described_class.flipper + described_class.send(:flipper) described_class.instance_variable_set(:@flipper, nil) - described_class.flipper + described_class.send(:flipper) end end end @@ -179,21 +179,21 @@ describe Feature, stub_feature_flags: false do expect(described_class.enabled?(:enabled_feature_flag)).to be_truthy end - it { expect(described_class.l1_cache_backend).to eq(Gitlab::ProcessMemoryCache.cache_backend) } - it { expect(described_class.l2_cache_backend).to eq(Rails.cache) } + it { expect(described_class.send(:l1_cache_backend)).to eq(Gitlab::ProcessMemoryCache.cache_backend) } + it { expect(described_class.send(:l2_cache_backend)).to eq(Rails.cache) } it 'caches the status in L1 and L2 caches', :request_store, :use_clean_rails_memory_store_caching do described_class.enable(:enabled_feature_flag) flipper_key = "flipper/v1/feature/enabled_feature_flag" - expect(described_class.l2_cache_backend) + expect(described_class.send(:l2_cache_backend)) .to receive(:fetch) .once .with(flipper_key, expires_in: 1.hour) .and_call_original - expect(described_class.l1_cache_backend) + expect(described_class.send(:l1_cache_backend)) .to receive(:fetch) .once .with(flipper_key, expires_in: 1.minute) @@ -215,14 +215,14 @@ describe Feature, stub_feature_flags: false do let(:flag) { :some_feature_flag } before do - described_class.flipper.memoize = false + described_class.send(:flipper).memoize = false described_class.enabled?(flag) end it 'caches the status in L1 cache for the first minute' do expect do - expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original - expect(described_class.l2_cache_backend).not_to receive(:fetch) + expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original + expect(described_class.send(:l2_cache_backend)).not_to receive(:fetch) expect(described_class.enabled?(flag)).to be_truthy end.not_to exceed_query_limit(0) end @@ -230,8 +230,8 @@ describe Feature, stub_feature_flags: false do it 'caches the status in L2 cache after 2 minutes' do Timecop.travel 2.minutes do expect do - expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original - expect(described_class.l2_cache_backend).to receive(:fetch).once.and_call_original + expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original + expect(described_class.send(:l2_cache_backend)).to receive(:fetch).once.and_call_original expect(described_class.enabled?(flag)).to be_truthy end.not_to exceed_query_limit(0) end @@ -240,8 +240,8 @@ describe Feature, stub_feature_flags: false do it 'fetches the status after an hour' do Timecop.travel 61.minutes do expect do - expect(described_class.l1_cache_backend).to receive(:fetch).once.and_call_original - expect(described_class.l2_cache_backend).to receive(:fetch).once.and_call_original + expect(described_class.send(:l1_cache_backend)).to receive(:fetch).once.and_call_original + expect(described_class.send(:l2_cache_backend)).to receive(:fetch).once.and_call_original expect(described_class.enabled?(flag)).to be_truthy end.not_to exceed_query_limit(1) end diff --git a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb index f82e49f9323..ea04862ed74 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/sequence_spec.rb @@ -56,11 +56,24 @@ describe Gitlab::Ci::Pipeline::Chain::Sequence do end it 'adds sequence duration to duration histogram' do - allow(command).to receive(:duration_histogram).and_return(histogram) + allow(command.metrics) + .to receive(:pipeline_creation_duration_histogram) + .and_return(histogram) subject.build! expect(histogram).to have_received(:observe) end + + it 'records pipeline size by pipeline source in a histogram' do + allow(command.metrics) + .to receive(:pipeline_size_histogram) + .and_return(histogram) + + subject.build! + + expect(histogram).to have_received(:observe) + .with({ source: 'push' }, 0) + end end end diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index bae6a19a54a..f6e6c031624 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -11,7 +11,7 @@ describe Gitlab::Experimentation do } }) - Feature.get(:test_experiment_experiment_percentage).enable_percentage_of_time(enabled_percentage) + Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage) end let(:environment) { Rails.env.test? } diff --git a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb index f86a5df7e11..8339006fe9f 100644 --- a/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb +++ b/spec/lib/gitlab/git/rugged_impl/use_rugged_spec.rb @@ -8,7 +8,6 @@ describe Gitlab::Git::RuggedImpl::UseRugged, :seed_helper do let(:project) { create(:project, :repository) } let(:repository) { project.repository } let(:feature_flag_name) { 'feature-flag-name' } - let(:feature_flag) { Feature.get(feature_flag_name) } let(:temp_gitaly_metadata_file) { create_temporary_gitaly_metadata_file } before(:all) do diff --git a/spec/lib/gitlab/jira_import/base_importer_spec.rb b/spec/lib/gitlab/jira_import/base_importer_spec.rb index 3bccb2395d6..cda491393e8 100644 --- a/spec/lib/gitlab/jira_import/base_importer_spec.rb +++ b/spec/lib/gitlab/jira_import/base_importer_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::JiraImport::BaseImporter do before do stub_jira_service_test - allow(project).to receive(:validate_jira_import_settings!) + allow(Gitlab::JiraImport).to receive(:validate_project_settings!) end context 'when Jira service exists' do diff --git a/spec/lib/gitlab/jira_import_spec.rb b/spec/lib/gitlab/jira_import_spec.rb index c5c3d6ef4b9..6272f913318 100644 --- a/spec/lib/gitlab/jira_import_spec.rb +++ b/spec/lib/gitlab/jira_import_spec.rb @@ -5,6 +5,95 @@ require 'spec_helper' describe Gitlab::JiraImport do let(:project_id) { 321 } + describe '.validate_project_settings!' do + include JiraServiceHelper + + let_it_be(:project, reload: true) { create(:project) } + + shared_examples 'raise Jira import error' do |message| + it 'returns error' do + expect { subject }.to raise_error(Projects::ImportService::Error, message) + end + end + + shared_examples 'jira configuration base checks' do + context 'when Jira service was not setup' do + it_behaves_like 'raise Jira import error', 'Jira integration not configured.' + end + + context 'when Jira service exists' do + let!(:jira_service) { create(:jira_service, project: project, active: true) } + + context 'when Jira connection is not valid' do + before do + WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo') + .to_raise(JIRA::HTTPError.new(double(message: 'Some failure.'))) + end + + it_behaves_like 'raise Jira import error', 'Unable to connect to the Jira instance. Please check your Jira integration configuration.' + end + end + end + + before do + stub_jira_service_test + end + + context 'without user param' do + subject { described_class.validate_project_settings!(project) } + + it_behaves_like 'jira configuration base checks' + + context 'when jira connection is valid' do + let!(:jira_service) { create(:jira_service, project: project, active: true) } + + it 'does not return any error' do + expect { subject }.not_to raise_error + end + end + end + + context 'with user param provided' do + let_it_be(:user) { create(:user) } + + subject { described_class.validate_project_settings!(project, user: user) } + + context 'when user has permission to run import' do + before do + project.add_maintainer(user) + end + + it_behaves_like 'jira configuration base checks' + + context 'when jira service is configured' do + let!(:jira_service) { create(:jira_service, project: project, active: true) } + + context 'when issues feature is disabled' do + let_it_be(:project, reload: true) { create(:project, :issues_disabled) } + + it_behaves_like 'raise Jira import error', 'Cannot import because issues are not available in this project.' + end + + context 'when everything is ok' do + it 'does not return any error' do + expect { subject }.not_to raise_error + end + end + end + end + + context 'when user does not have permissions to run the import' do + before do + create(:jira_service, project: project, active: true) + + project.add_developer(user) + end + + it_behaves_like 'raise Jira import error', 'You do not have permissions to run the import.' + end + end + end + describe '.jira_issue_cache_key' do it 'returns cache key for Jira issue imported to given project' do expect(described_class.jira_issue_cache_key(project_id, 'DEMO-123')).to eq("jira-import/items-mapper/#{project_id}/issues/DEMO-123") diff --git a/spec/lib/gitlab/metrics/method_call_spec.rb b/spec/lib/gitlab/metrics/method_call_spec.rb index 229db67ec88..035d875258c 100644 --- a/spec/lib/gitlab/metrics/method_call_spec.rb +++ b/spec/lib/gitlab/metrics/method_call_spec.rb @@ -26,7 +26,7 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is enabled' do before do - Feature.get(:prometheus_metrics_method_instrumentation).enable + stub_feature_flags(prometheus_metrics_method_instrumentation: true) end around do |example| @@ -50,7 +50,7 @@ describe Gitlab::Metrics::MethodCall do context 'prometheus instrumentation is disabled' do before do - Feature.get(:prometheus_metrics_method_instrumentation).disable + stub_feature_flags(prometheus_metrics_method_instrumentation: false) end it 'observes using NullMetric' do diff --git a/spec/lib/gitlab/metrics/methods_spec.rb b/spec/lib/gitlab/metrics/methods_spec.rb index bca94deb1d8..5cf8db55142 100644 --- a/spec/lib/gitlab/metrics/methods_spec.rb +++ b/spec/lib/gitlab/metrics/methods_spec.rb @@ -104,7 +104,7 @@ describe Gitlab::Metrics::Methods do context 'when feature is enabled' do before do - Feature.get(feature_name).enable + stub_feature_flags(feature_name => true) end it "initializes #{metric_type} metric" do @@ -118,7 +118,7 @@ describe Gitlab::Metrics::Methods do context 'when feature is disabled' do before do - Feature.get(feature_name).disable + stub_feature_flags(feature_name => false) end it "returns NullMetric" do diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb new file mode 100644 index 00000000000..317420c0f2c --- /dev/null +++ b/spec/models/integration_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integration do + let(:project_1) { create(:project) } + let(:project_2) { create(:project) } + let(:instance_integration) { create(:jira_service, :instance) } + + before do + create(:jira_service, project: project_1, inherit_from_id: instance_integration.id) + create(:jira_service, project: project_2, inherit_from_id: nil) + create(:slack_service, project: project_1, inherit_from_id: nil) + end + + describe '#with_custom_integration_compared_to' do + it 'returns projects with custom integrations' do + expect(Project.with_custom_integration_compared_to(instance_integration)).to contain_exactly(project_2) + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 6c988719c1a..5f4a3dbd99c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -4262,7 +4262,7 @@ describe Project do describe '#auto_devops_enabled?' do before do - Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(0) + Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 0) end let_it_be(:project, reload: true) { create(:project) } @@ -4464,7 +4464,7 @@ describe Project do let_it_be(:project, reload: true) { create(:project) } before do - Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(0) + Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 0) end context 'when explicitly disabled' do @@ -4510,7 +4510,7 @@ describe Project do before do create(:project_auto_devops, project: project, enabled: false) - Feature.get(:force_autodevops_on_by_default).enable_percentage_of_actors(100) + Feature.enable_percentage_of_actors(:force_autodevops_on_by_default, 100) end it 'does not have auto devops implicitly disabled' do @@ -5999,99 +5999,6 @@ describe Project do end end - describe '#validate_jira_import_settings!' do - include JiraServiceHelper - - let_it_be(:project, reload: true) { create(:project) } - - shared_examples 'raise Jira import error' do |message| - it 'returns error' do - expect { subject }.to raise_error(Projects::ImportService::Error, message) - end - end - - shared_examples 'jira configuration base checks' do - context 'when Jira service was not setup' do - it_behaves_like 'raise Jira import error', 'Jira integration not configured.' - end - - context 'when Jira service exists' do - let!(:jira_service) { create(:jira_service, project: project, active: true) } - - context 'when Jira connection is not valid' do - before do - WebMock.stub_request(:get, 'https://jira.example.com/rest/api/2/serverInfo') - .to_raise(JIRA::HTTPError.new(double(message: 'Some failure.'))) - end - - it_behaves_like 'raise Jira import error', 'Unable to connect to the Jira instance. Please check your Jira integration configuration.' - end - end - end - - before do - stub_jira_service_test - end - - context 'without user param' do - subject { project.validate_jira_import_settings! } - - it_behaves_like 'jira configuration base checks' - - context 'when jira connection is valid' do - let!(:jira_service) { create(:jira_service, project: project, active: true) } - - it 'does not return any error' do - expect { subject }.not_to raise_error - end - end - end - - context 'with user param provided' do - let_it_be(:user) { create(:user) } - - subject { project.validate_jira_import_settings!(user: user) } - - context 'when user has permission to run import' do - before do - project.add_maintainer(user) - end - - it_behaves_like 'jira configuration base checks' - end - - context 'when user does not have permissions to run the import' do - before do - create(:jira_service, project: project, active: true) - - project.add_developer(user) - end - - it_behaves_like 'raise Jira import error', 'You do not have permissions to run the import.' - end - - context 'when user has permission to run import' do - before do - project.add_maintainer(user) - end - - let!(:jira_service) { create(:jira_service, project: project, active: true) } - - context 'when issues feature is disabled' do - let_it_be(:project, reload: true) { create(:project, :issues_disabled) } - - it_behaves_like 'raise Jira import error', 'Cannot import because issues are not available in this project.' - end - - context 'when everything is ok' do - it 'does not return any error' do - expect { subject }.not_to raise_error - end - end - end - end - end - describe '#design_management_enabled?' do let(:project) { build(:project) } diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index 83739efe14c..59a9ed2f77d 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -39,9 +39,9 @@ describe API::Features, stub_feature_flags: false do end before do - Feature.get('feature_1').enable - Feature.get('feature_2').disable - Feature.get('feature_3').enable Feature.group(:perf_team) + Feature.enable('feature_1') + Feature.disable('feature_2') + Feature.enable('feature_3', Feature.group(:perf_team)) end it 'returns a 401 for anonymous users' do @@ -227,10 +227,8 @@ describe API::Features, stub_feature_flags: false do end context 'when the feature exists' do - let(:feature) { Feature.get(feature_name) } - before do - feature.disable # This also persists the feature on the DB + Feature.disable(feature_name) # This also persists the feature on the DB end context 'when passed value=true' do @@ -273,8 +271,8 @@ describe API::Features, stub_feature_flags: false do context 'when feature is enabled and value=false is passed' do it 'disables the feature' do - feature.enable - expect(feature).to be_enabled + Feature.enable(feature_name) + expect(Feature.enabled?(feature_name)).to eq(true) post api("/features/#{feature_name}", admin), params: { value: 'false' } @@ -286,8 +284,8 @@ describe API::Features, stub_feature_flags: false do end it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do - feature.enable(Feature.group(:perf_team)) - expect(Feature.get(feature_name).enabled?(admin)).to be_truthy + Feature.enable(feature_name, Feature.group(:perf_team)) + expect(Feature.enabled?(feature_name, admin)).to be_truthy post api("/features/#{feature_name}", admin), params: { value: 'false', feature_group: 'perf_team' } @@ -299,8 +297,8 @@ describe API::Features, stub_feature_flags: false do end it 'disables the feature for the given user when passed user=username' do - feature.enable(user) - expect(Feature.get(feature_name).enabled?(user)).to be_truthy + Feature.enable(feature_name, user) + expect(Feature.enabled?(feature_name, user)).to be_truthy post api("/features/#{feature_name}", admin), params: { value: 'false', user: user.username } @@ -314,7 +312,7 @@ describe API::Features, stub_feature_flags: false do context 'with a pre-existing percentage of time value' do before do - feature.enable_percentage_of_time(50) + Feature.enable_percentage_of_time(feature_name, 50) end it 'updates the percentage of time if passed an integer' do @@ -333,7 +331,7 @@ describe API::Features, stub_feature_flags: false do context 'with a pre-existing percentage of actors value' do before do - feature.enable_percentage_of_actors(42) + Feature.enable_percentage_of_actors(feature_name, 42) end it 'updates the percentage of actors if passed an integer' do @@ -378,14 +376,17 @@ describe API::Features, stub_feature_flags: false do context 'when the gate value was set' do before do - Feature.get(feature_name).enable + Feature.enable(feature_name) end it 'deletes an enabled feature' do - delete api("/features/#{feature_name}", admin) + expect do + delete api("/features/#{feature_name}", admin) + Feature.reset + end.to change { Feature.persisted_name?(feature_name) } + .and change { Feature.enabled?(feature_name) } expect(response).to have_gitlab_http_status(:no_content) - expect(Feature.get(feature_name)).not_to be_enabled end end end diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 0b8c7eef7c1..e6dd1fecb69 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -52,7 +52,7 @@ describe API::Settings, 'Settings' do storages = Gitlab.config.repositories.storages .merge({ 'custom' => 'tmp/tests/custom_repositories' }) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) - Feature.get(:sourcegraph).enable + stub_feature_flags(sourcegraph: true) end it "updates application settings" do diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 681ce9669e2..b9456d5fcd4 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -77,6 +77,18 @@ describe Ci::CreatePipelineService do pipeline end + it 'records pipeline size in a prometheus histogram' do + histogram = spy('pipeline size histogram') + + allow(Gitlab::Ci::Pipeline::Chain::Metrics) + .to receive(:new).and_return(histogram) + + execute_service + + expect(histogram).to have_received(:observe) + .with({ source: 'push' }, 5) + end + context 'when merge requests already exist for this source branch' do let(:merge_request_1) do create(:merge_request, source_branch: 'feature', target_branch: "master", source_project: project) diff --git a/spec/services/discussions/resolve_service_spec.rb b/spec/services/discussions/resolve_service_spec.rb index dabd33bfd5e..7461934b455 100644 --- a/spec/services/discussions/resolve_service_spec.rb +++ b/spec/services/discussions/resolve_service_spec.rb @@ -6,7 +6,7 @@ describe Discussions::ResolveService do describe '#execute' do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user, developer_projects: [project]) } - let_it_be(:merge_request) { create(:merge_request, source_project: project) } + let_it_be(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds, source_project: project) } let(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } let(:service) { described_class.new(project, user, one_or_more_discussions: discussion) } @@ -32,6 +32,36 @@ describe Discussions::ResolveService do service.execute end + it 'schedules an auto-merge' do + expect(AutoMergeProcessWorker).to receive(:perform_async).with(discussion.noteable.id) + + service.execute + end + + context 'with a project that requires all discussion to be resolved' do + before do + project.update(only_allow_merge_if_all_discussions_are_resolved: true) + end + + after do + project.update(only_allow_merge_if_all_discussions_are_resolved: false) + end + + let_it_be(:other_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } + + it 'does not schedule an auto-merge' do + expect(AutoMergeProcessWorker).not_to receive(:perform_async) + + service.execute + end + + it 'schedules an auto-merge' do + expect(AutoMergeProcessWorker).to receive(:perform_async) + + described_class.new(project, user, one_or_more_discussions: [discussion, other_discussion]).execute + end + end + it 'adds a system note to the discussion' do issue = create(:issue, project: project) @@ -70,6 +100,12 @@ describe Discussions::ResolveService do service.execute end + + it 'does not schedule an auto-merge' do + expect(AutoMergeProcessWorker).not_to receive(:perform_async) + + service.execute + end end end end diff --git a/spec/services/jira_import/start_import_service_spec.rb b/spec/services/jira_import/start_import_service_spec.rb index 61edb86ac90..9dc8cdb1475 100644 --- a/spec/services/jira_import/start_import_service_spec.rb +++ b/spec/services/jira_import/start_import_service_spec.rb @@ -13,7 +13,7 @@ describe JiraImport::StartImportService do context 'when an error is returned from the project validation' do before do - allow(project).to receive(:validate_jira_import_settings!) + allow(Gitlab::JiraImport).to receive(:validate_project_settings!) .and_raise(Projects::ImportService::Error, 'Jira import feature is disabled.') end @@ -25,7 +25,7 @@ describe JiraImport::StartImportService do before do stub_jira_service_test - allow(project).to receive(:validate_jira_import_settings!) + allow(Gitlab::JiraImport).to receive(:validate_project_settings!) end context 'when Jira project key is not provided' do diff --git a/spec/support/helpers/stub_feature_flags.rb b/spec/support/helpers/stub_feature_flags.rb index 9a298bd3d83..696148cacaf 100644 --- a/spec/support/helpers/stub_feature_flags.rb +++ b/spec/support/helpers/stub_feature_flags.rb @@ -47,7 +47,7 @@ module StubFeatureFlags def stub_feature_flags(features) features.each do |feature_name, actors| # Remove feature flag overwrite - feature = Feature.get(feature_name) + feature = Feature.get(feature_name) # rubocop:disable Gitlab/AvoidFeatureGet feature.remove Array(actors).each do |actor| diff --git a/spec/views/profiles/preferences/show.html.haml_spec.rb b/spec/views/profiles/preferences/show.html.haml_spec.rb index 16e4bd9c6d1..2e50e329cfd 100644 --- a/spec/views/profiles/preferences/show.html.haml_spec.rb +++ b/spec/views/profiles/preferences/show.html.haml_spec.rb @@ -32,8 +32,7 @@ describe 'profiles/preferences/show' do end before do - # Can't use stub_feature_flags because we use Feature.get to check if conditinally applied - Feature.get(:sourcegraph).enable sourcegraph_feature + stub_feature_flags(sourcegraph: sourcegraph_feature) stub_application_setting(sourcegraph_enabled: sourcegraph_enabled) end diff --git a/yarn.lock b/yarn.lock index d1a0a28f1b8..11ed66a2679 100644 --- a/yarn.lock +++ b/yarn.lock @@ -782,10 +782,10 @@ eslint-plugin-vue "^6.2.1" vue-eslint-parser "^7.0.0" -"@gitlab/svgs@1.131.0": - version "1.131.0" - resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.131.0.tgz#4fc3e15cea899e8941954197bfc600afd6d9e210" - integrity sha512-c/0FWIEz3fJjC0n2XdxQ/XdGzpJqPwiye6fG+imA0HRavlg+C0xENzUif8A+1t5/zNIkUCTRugNbknfpU7Y7DA== +"@gitlab/svgs@1.133.0": + version "1.133.0" + resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.133.0.tgz#737fab044fc03cdb7690b5cf8c4f7a1a9222b43a" + integrity sha512-Briyqx73AuUafsmN3fj8nGIMG5jzA5gZtncs9UhnlgqA02oasdgBCNujD7cAUKDV2++sgFcMxkq90GTis+bkwQ== "@gitlab/ui@16.1.0": version "16.1.0" @@ -4219,6 +4219,13 @@ entities@^2.0.0: resolved "https://registry.yarnpkg.com/entities/-/entities-2.0.0.tgz#68d6084cab1b079767540d80e56a39b423e4abf4" integrity sha512-D9f7V0JSRwIxlRI2mjMqufDrRDnx8p+eEOz7aUM9SuvF8gsBzra0/6tbjl1m8eQHrZlYj6PxqE00hZ1SAIKPLw== +entity-decode@^2.0.2: + version "2.0.2" + resolved "https://registry.yarnpkg.com/entity-decode/-/entity-decode-2.0.2.tgz#e4f807e52c3294246e9347d1f2b02b07fd5f92e7" + integrity sha512-5CCY/3ci4MC1m2jlumNjWd7VBFt4VfFnmSqSNmVcXq4gxM3Vmarxtt+SvmBnzwLS669MWdVuXboNVj1qN2esVg== + dependencies: + he "^1.1.1" + errno@^0.1.3, errno@~0.1.7: version "0.1.7" resolved "https://registry.yarnpkg.com/errno/-/errno-0.1.7.tgz#4684d71779ad39af177e3f007996f7c67c852618" @@ -5568,7 +5575,7 @@ hash.js@^1.0.0, hash.js@^1.0.3: inherits "^2.0.3" minimalistic-assert "^1.0.0" -he@^1.1.0, he@^1.2.0: +he@^1.1.0, he@^1.1.1, he@^1.2.0: version "1.2.0" resolved "https://registry.yarnpkg.com/he/-/he-1.2.0.tgz#84ae65fa7eafb165fddb61566ae14baf05664f0f" integrity sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw== @@ -7750,19 +7757,19 @@ merge2@^1.2.3: resolved "https://registry.yarnpkg.com/merge2/-/merge2-1.2.3.tgz#7ee99dbd69bb6481689253f018488a1b902b0ed5" integrity sha512-gdUU1Fwj5ep4kplwcmftruWofEFt6lfpkkr3h860CXbAB9c3hGb55EOL2ali0Td5oebvW0E1+3Sr+Ur7XfKpRA== -mermaid@^8.4.8: - version "8.4.8" - resolved "https://registry.yarnpkg.com/mermaid/-/mermaid-8.4.8.tgz#8adcfdbc505d6bca52df167cff690427c9727b60" - integrity sha512-sumTNBFwMX7oMQgogdr3NhgTeQOiwcEsm23rQ4KHGW7tpmvMwER1S+1gjCSSnqlmM/zw7Ga7oesYCYicKboRwQ== +mermaid@^8.5.1: + version "8.5.1" + resolved "https://registry.yarnpkg.com/mermaid/-/mermaid-8.5.1.tgz#c3b3a39d14315c8b00ba43c047101c2d7cf463f1" + integrity sha512-CU8jEceILn/zS5Fs6CxxAIilhXoQd9b3Hy/UR/ZIioK75yyLf8f+V8WX2Voq4oaXiwExCwYUcBA9WV4oyr31wA== dependencies: "@braintree/sanitize-url" "^3.1.0" crypto-random-string "^3.0.1" d3 "^5.7.0" dagre "^0.8.4" dagre-d3 "^0.6.4" + entity-decode "^2.0.2" graphlib "^2.1.7" he "^1.2.0" - lodash "^4.17.11" minify "^4.1.1" moment-mini "^2.22.1" scope-css "^1.2.1"