From 045e4edc1abefe4c3c9d4a8064683f5414884b9f Mon Sep 17 00:00:00 2001 From: Rune Philosof Date: Wed, 22 Feb 2017 09:22:54 +0000 Subject: Enable the import_url field. Closes #28531 The field should be enabled on `project/import/new`. It shouldn't hurt to have the field enabled on `projects/new` as well. --- app/views/shared/_import_form.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/_import_form.html.haml b/app/views/shared/_import_form.html.haml index 54b5ae2402e..1c7c73be933 100644 --- a/app/views/shared/_import_form.html.haml +++ b/app/views/shared/_import_form.html.haml @@ -2,7 +2,7 @@ = f.label :import_url, class: 'control-label' do %span Git repository URL .col-sm-10 - = f.text_field :import_url, autocomplete: 'off', class: 'form-control', placeholder: 'https://username:password@gitlab.company.com/group/project.git', disabled: true + = f.text_field :import_url, autocomplete: 'off', class: 'form-control', placeholder: 'https://username:password@gitlab.company.com/group/project.git' .well.prepend-top-20 %ul -- cgit v1.2.1 From 3e1bd5570f0ca277fb82a45d83f5bf44f3663a0d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 22 Mar 2017 22:31:07 +0800 Subject: Test both PostgreSQL and MySQL for the win. --- .gitlab-ci.yml | 219 +++++++++++++++++++++++++++++------------ config/database.yml.postgresql | 5 +- scripts/prepare_build.sh | 48 +++++---- scripts/utils.sh | 14 +++ 4 files changed, 202 insertions(+), 84 deletions(-) create mode 100644 scripts/utils.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 24627b5faca..51fa8a6862e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.3-git-2.7-phantomjs-2.1-node-7.1" +image: "registry.gitlab.com/godfat/gitlab-build-images:ruby-2.3.3-git-2.7-phantomjs-2.1-node-7.1-postgresql-9.6" cache: key: "ruby-233" @@ -9,19 +9,14 @@ variables: MYSQL_ALLOW_EMPTY_PASSWORD: "1" RAILS_ENV: "test" SIMPLECOV: "true" - SETUP_DB: "true" - USE_BUNDLE_INSTALL: "true" GIT_DEPTH: "20" PHANTOMJS_VERSION: "2.1.1" GET_SOURCES_ATTEMPTS: "3" before_script: - - source ./scripts/prepare_build.sh - - cp config/gitlab.yml.example config/gitlab.yml - bundle --version - - '[ "$USE_BUNDLE_INSTALL" != "true" ] || retry bundle install --without postgres production --jobs $(nproc) --clean $FLAGS' - - retry gem install knapsack - - '[ "$SETUP_DB" != "true" ] || bundle exec rake db:drop db:create db:schema:load db:migrate add_limits_mysql' + - . scripts/utils.sh + - ./scripts/prepare_build.sh stages: - prepare @@ -48,19 +43,27 @@ stages: paths: - knapsack/ -.use-db: &use-db +.use-pg: &use-pg + services: + - postgres:latest + - redis:alpine + variables: + GITLAB_DATABASE: "postgresql" + +.use-mysql: &use-mysql services: - mysql:latest - redis:alpine + variables: + GITLAB_DATABASE: "mysql" .rspec-knapsack: &rspec-knapsack stage: test <<: *dedicated-runner - <<: *use-db script: - JOB_NAME=( $CI_JOB_NAME ) - - export CI_NODE_INDEX=${JOB_NAME[1]} - - export CI_NODE_TOTAL=${JOB_NAME[2]} + - export CI_NODE_INDEX=${JOB_NAME[-2]} + - export CI_NODE_TOTAL=${JOB_NAME[-1]} - export KNAPSACK_REPORT_PATH=knapsack/rspec_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - export KNAPSACK_GENERATE_REPORT=true - cp knapsack/rspec_report.json ${KNAPSACK_REPORT_PATH} @@ -73,14 +76,21 @@ stages: - knapsack/ - tmp/capybara/ +.rspec-knapsack-pg: &rspec-knapsack-pg + <<: *rspec-knapsack + <<: *use-pg + +.rspec-knapsack-mysql: &rspec-knapsack-mysql + <<: *rspec-knapsack + <<: *use-mysql + .spinach-knapsack: &spinach-knapsack stage: test <<: *dedicated-runner - <<: *use-db script: - JOB_NAME=( $CI_JOB_NAME ) - - export CI_NODE_INDEX=${JOB_NAME[1]} - - export CI_NODE_TOTAL=${JOB_NAME[2]} + - export CI_NODE_INDEX=${JOB_NAME[-2]} + - export CI_NODE_TOTAL=${JOB_NAME[-1]} - export KNAPSACK_REPORT_PATH=knapsack/spinach_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - export KNAPSACK_GENERATE_REPORT=true - cp knapsack/spinach_report.json ${KNAPSACK_REPORT_PATH} @@ -93,6 +103,14 @@ stages: - knapsack/ - tmp/capybara/ +.spinach-knapsack-pg: &spinach-knapsack-pg + <<: *spinach-knapsack + <<: *use-pg + +.spinach-knapsack-mysql: &spinach-knapsack-mysql + <<: *spinach-knapsack + <<: *use-mysql + # Prepare and merge knapsack tests knapsack: @@ -105,7 +123,7 @@ knapsack: - '[[ -f knapsack/spinach_report.json ]] || echo "{}" > knapsack/spinach_report.json' setup-test-env: - <<: *use-db + <<: *use-pg <<: *dedicated-runner stage: prepare script: @@ -136,37 +154,69 @@ update-knapsack: - master@gitlab/gitlabhq - master@gitlab/gitlab-ee -rspec 0 20: *rspec-knapsack -rspec 1 20: *rspec-knapsack -rspec 2 20: *rspec-knapsack -rspec 3 20: *rspec-knapsack -rspec 4 20: *rspec-knapsack -rspec 5 20: *rspec-knapsack -rspec 6 20: *rspec-knapsack -rspec 7 20: *rspec-knapsack -rspec 8 20: *rspec-knapsack -rspec 9 20: *rspec-knapsack -rspec 10 20: *rspec-knapsack -rspec 11 20: *rspec-knapsack -rspec 12 20: *rspec-knapsack -rspec 13 20: *rspec-knapsack -rspec 14 20: *rspec-knapsack -rspec 15 20: *rspec-knapsack -rspec 16 20: *rspec-knapsack -rspec 17 20: *rspec-knapsack -rspec 18 20: *rspec-knapsack -rspec 19 20: *rspec-knapsack - -spinach 0 10: *spinach-knapsack -spinach 1 10: *spinach-knapsack -spinach 2 10: *spinach-knapsack -spinach 3 10: *spinach-knapsack -spinach 4 10: *spinach-knapsack -spinach 5 10: *spinach-knapsack -spinach 6 10: *spinach-knapsack -spinach 7 10: *spinach-knapsack -spinach 8 10: *spinach-knapsack -spinach 9 10: *spinach-knapsack +rspec pg 0 20: *rspec-knapsack-pg +rspec pg 1 20: *rspec-knapsack-pg +rspec pg 2 20: *rspec-knapsack-pg +rspec pg 3 20: *rspec-knapsack-pg +rspec pg 4 20: *rspec-knapsack-pg +rspec pg 5 20: *rspec-knapsack-pg +rspec pg 6 20: *rspec-knapsack-pg +rspec pg 7 20: *rspec-knapsack-pg +rspec pg 8 20: *rspec-knapsack-pg +rspec pg 9 20: *rspec-knapsack-pg +rspec pg 10 20: *rspec-knapsack-pg +rspec pg 11 20: *rspec-knapsack-pg +rspec pg 12 20: *rspec-knapsack-pg +rspec pg 13 20: *rspec-knapsack-pg +rspec pg 14 20: *rspec-knapsack-pg +rspec pg 15 20: *rspec-knapsack-pg +rspec pg 16 20: *rspec-knapsack-pg +rspec pg 17 20: *rspec-knapsack-pg +rspec pg 18 20: *rspec-knapsack-pg +rspec pg 19 20: *rspec-knapsack-pg + +rspec mysql 0 20: *rspec-knapsack-mysql +rspec mysql 1 20: *rspec-knapsack-mysql +rspec mysql 2 20: *rspec-knapsack-mysql +rspec mysql 3 20: *rspec-knapsack-mysql +rspec mysql 4 20: *rspec-knapsack-mysql +rspec mysql 5 20: *rspec-knapsack-mysql +rspec mysql 6 20: *rspec-knapsack-mysql +rspec mysql 7 20: *rspec-knapsack-mysql +rspec mysql 8 20: *rspec-knapsack-mysql +rspec mysql 9 20: *rspec-knapsack-mysql +rspec mysql 10 20: *rspec-knapsack-mysql +rspec mysql 11 20: *rspec-knapsack-mysql +rspec mysql 12 20: *rspec-knapsack-mysql +rspec mysql 13 20: *rspec-knapsack-mysql +rspec mysql 14 20: *rspec-knapsack-mysql +rspec mysql 15 20: *rspec-knapsack-mysql +rspec mysql 16 20: *rspec-knapsack-mysql +rspec mysql 17 20: *rspec-knapsack-mysql +rspec mysql 18 20: *rspec-knapsack-mysql +rspec mysql 19 20: *rspec-knapsack-mysql + +spinach pg 0 10: *spinach-knapsack-pg +spinach pg 1 10: *spinach-knapsack-pg +spinach pg 2 10: *spinach-knapsack-pg +spinach pg 3 10: *spinach-knapsack-pg +spinach pg 4 10: *spinach-knapsack-pg +spinach pg 5 10: *spinach-knapsack-pg +spinach pg 6 10: *spinach-knapsack-pg +spinach pg 7 10: *spinach-knapsack-pg +spinach pg 8 10: *spinach-knapsack-pg +spinach pg 9 10: *spinach-knapsack-pg + +spinach mysql 0 10: *spinach-knapsack-mysql +spinach mysql 1 10: *spinach-knapsack-mysql +spinach mysql 2 10: *spinach-knapsack-mysql +spinach mysql 3 10: *spinach-knapsack-mysql +spinach mysql 4 10: *spinach-knapsack-mysql +spinach mysql 5 10: *spinach-knapsack-mysql +spinach mysql 6 10: *spinach-knapsack-mysql +spinach mysql 7 10: *spinach-knapsack-mysql +spinach mysql 8 10: *spinach-knapsack-mysql +spinach mysql 9 10: *spinach-knapsack-mysql # Other generic tests .ruby-static-analysis: &ruby-static-analysis @@ -217,29 +267,43 @@ rake ee_compat_check: paths: - ee_compat_check/patches/*.patch -rake db:migrate:reset: +.db-migrate-reset: &db-migrate-reset stage: test - <<: *use-db <<: *dedicated-runner script: - bundle exec rake db:migrate:reset -rake db:rollback: +pg rake db:migrate:reset: + <<: *db-migrate-reset + <<: *use-pg + +mysql rake db:migrate:reset: + <<: *db-migrate-reset + <<: *use-mysql + +.db-rollback: &db-rollback stage: test - <<: *use-db <<: *dedicated-runner script: - bundle exec rake db:rollback STEP=120 - bundle exec rake db:migrate -rake db:seed_fu: +pg rake db:rollback: + <<: *db-rollback + <<: *use-pg + +mysql rake db:rollback: + <<: *db-rollback + <<: *use-mysql + +.db-seed_fu-variables: &db-seed_fu-variables + SIZE: "1" + SETUP_DB: "false" + RAILS_ENV: "development" + +.db-seed_fu: &db-seed_fu stage: test - <<: *use-db <<: *dedicated-runner - variables: - SIZE: "1" - SETUP_DB: "false" - RAILS_ENV: "development" script: - git clone https://gitlab.com/gitlab-org/gitlab-test.git /home/git/repositories/gitlab-org/gitlab-test.git @@ -250,6 +314,20 @@ rake db:seed_fu: paths: - log/development.log +pg rake db:seed_fu: + <<: *db-seed_fu + <<: *use-pg + variables: + <<: *db-seed_fu-variables + GITLAB_DATABASE: "postgresql" + +mysql rake db:seed_fu: + <<: *db-seed_fu + <<: *use-mysql + variables: + <<: *db-seed_fu-variables + GITLAB_DATABASE: "mysql" + rake gitlab:assets:compile: stage: test <<: *dedicated-runner @@ -275,7 +353,7 @@ rake karma: - vendor/ruby - node_modules stage: test - <<: *use-db + <<: *use-pg <<: *dedicated-runner variables: BABEL_ENV: "coverage" @@ -314,12 +392,9 @@ bundler:audit: script: - "bundle exec bundle-audit check --update" -migration paths: +.migration-paths: &migration-paths stage: test - <<: *use-db <<: *dedicated-runner - variables: - SETUP_DB: "false" only: - master@gitlab-org/gitlab-ce - master@gitlab-org/gitlab-ee @@ -330,13 +405,27 @@ migration paths: - git checkout -f FETCH_HEAD - cp config/resque.yml.example config/resque.yml - sed -i 's/localhost/redis/g' config/resque.yml - - bundle install --without postgres production --jobs $(nproc) $FLAGS --retry=3 + - bundle install --without production --jobs $(nproc) $FLAGS --retry=3 - bundle exec rake db:drop db:create db:schema:load db:seed_fu - git checkout $CI_COMMIT_SHA - - bundle install --without postgres production --jobs $(nproc) $FLAGS --retry=3 + - bundle install --without production --jobs $(nproc) $FLAGS --retry=3 - source scripts/prepare_build.sh - bundle exec rake db:migrate +pg migration paths: + <<: *migration-paths + <<: *use-pg + variables: + SETUP_DB: "false" + GITLAB_DATABASE: "postgresql" + +mysql migration paths: + <<: *migration-paths + <<: *use-mysql + variables: + SETUP_DB: "false" + GITLAB_DATABASE: "mysql" + coverage: stage: post-test services: [] @@ -388,7 +477,7 @@ trigger_docs: before_script: - apk update && apk add curl variables: - GIT_STRATEGY: none + GIT_STRATEGY: "none" cache: {} artifacts: {} script: diff --git a/config/database.yml.postgresql b/config/database.yml.postgresql index 7067e0fe402..c517a4c0cb8 100644 --- a/config/database.yml.postgresql +++ b/config/database.yml.postgresql @@ -9,7 +9,7 @@ production: # username: git # password: # host: localhost - # port: 5432 + # port: 5432 # # Development specific @@ -21,6 +21,7 @@ development: pool: 5 username: postgres password: + # host: localhost # # Staging specific @@ -32,6 +33,7 @@ staging: pool: 5 username: postgres password: + # host: localhost # Warning: The database defined as "test" will be erased and # re-generated from your development database when you run "rake". @@ -43,3 +45,4 @@ test: &test pool: 5 username: postgres password: + # host: localhost diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index 6e3f76b8399..822bb83e6c4 100755 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -1,25 +1,21 @@ #!/bin/sh -retry() { - if eval "$@"; then - return 0 - fi +. scripts/utils.sh - for i in 2 1; do - sleep 3s - echo "Retrying $i..." - if eval "$@"; then - return 0 - fi - done - return 1 -} +export SETUP_DB=${SETUP_DB:-true} +export GITLAB_DATABASE=${GITLAB_DATABASE:-postgresql} +export USE_BUNDLE_INSTALL=${USE_BUNDLE_INSTALL:-true} if [ -f /.dockerenv ] || [ -f ./dockerinit ]; then - cp config/database.yml.mysql config/database.yml - sed -i 's/username:.*/username: root/g' config/database.yml - sed -i 's/password:.*/password:/g' config/database.yml - sed -i 's/# socket:.*/host: mysql/g' config/database.yml + cp config/database.yml.$GITLAB_DATABASE config/database.yml + + if [ "$GITLAB_DATABASE" = 'postgresql' ]; then + sed -i 's/# host:.*/host: postgres/g' config/database.yml + else # assume it's mysql + sed -i 's/username:.*/username: root/g' config/database.yml + sed -i 's/password:.*/password:/g' config/database.yml + sed -i 's/# socket:.*/host: mysql/g' config/database.yml + fi cp config/resque.yml.example config/resque.yml sed -i 's/localhost/redis/g' config/resque.yml @@ -28,8 +24,24 @@ if [ -f /.dockerenv ] || [ -f ./dockerinit ]; then else rnd=$(awk 'BEGIN { srand() ; printf("%d\n",rand()*5) }') export PATH="$HOME/bin:/usr/local/bin:/usr/bin:/bin" - cp config/database.yml.mysql config/database.yml + cp config/database.yml.$GITLAB_DATABASE config/database.yml sed "s/username\:.*$/username\: runner/" -i config/database.yml sed "s/password\:.*$/password\: 'password'/" -i config/database.yml sed "s/gitlabhq_test/gitlabhq_test_$rnd/" -i config/database.yml fi + +cp config/gitlab.yml.example config/gitlab.yml + +if [ "$USE_BUNDLE_INSTALL" != "false" ]; then + retry bundle install --without production --jobs $(nproc) --clean $FLAGS +fi + +retry gem install knapsack + +if [ "$SETUP_DB" != "false" ]; then + bundle exec rake db:drop db:create db:schema:load db:migrate + + if [ "$GITLAB_DATABASE" = "mysql" ]; then + bundle exec rake add_limits_mysql + fi +fi diff --git a/scripts/utils.sh b/scripts/utils.sh new file mode 100644 index 00000000000..6faa701f0ce --- /dev/null +++ b/scripts/utils.sh @@ -0,0 +1,14 @@ +retry() { + if eval "$@"; then + return 0 + fi + + for i in 2 1; do + sleep 3s + echo "Retrying $i..." + if eval "$@"; then + return 0 + fi + done + return 1 +} -- cgit v1.2.1 From d0c44ee1e2d3a747a2092d60067cd96578089fb4 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 24 Mar 2017 05:27:02 +0800 Subject: Note that install knapsack later than bundle install Otherwise oddly some native gems could not be found under some circumstance. No idea why, hours wasted. --- scripts/prepare_build.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index 822bb83e6c4..885b7eabba0 100755 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -11,7 +11,7 @@ if [ -f /.dockerenv ] || [ -f ./dockerinit ]; then if [ "$GITLAB_DATABASE" = 'postgresql' ]; then sed -i 's/# host:.*/host: postgres/g' config/database.yml - else # assume it's mysql + else # Assume it's mysql sed -i 's/username:.*/username: root/g' config/database.yml sed -i 's/password:.*/password:/g' config/database.yml sed -i 's/# socket:.*/host: mysql/g' config/database.yml @@ -36,6 +36,8 @@ if [ "$USE_BUNDLE_INSTALL" != "false" ]; then retry bundle install --without production --jobs $(nproc) --clean $FLAGS fi +# Only install knapsack after bundle install! Otherwise oddly some native +# gems could not be found under some circumstance. No idea why, hours wasted. retry gem install knapsack if [ "$SETUP_DB" != "false" ]; then -- cgit v1.2.1 From 03cb7935187955ad68a4304a562a94a2eb1f246d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 24 Mar 2017 17:02:22 +0800 Subject: Test all on EE, only master on CE, and -mysql branches --- .gitlab-ci.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 51fa8a6862e..997d8268008 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -57,6 +57,16 @@ stages: variables: GITLAB_DATABASE: "mysql" +.only-master-and-ee-or-mysql: &only-master-and-ee-or-mysql + only: + - /\-(?i)mysql$/ + - master@gitlab-org/gitlab-ce + - master@gitlab/gitlabhq + - tags@gitlab-org/gitlab-ce + - tags@gitlab/gitlabhq + - //@gitlab-org/gitlab-ee + - //@gitlab/gitlab-ee + .rspec-knapsack: &rspec-knapsack stage: test <<: *dedicated-runner @@ -83,6 +93,7 @@ stages: .rspec-knapsack-mysql: &rspec-knapsack-mysql <<: *rspec-knapsack <<: *use-mysql + <<: *only-master-and-ee-or-mysql .spinach-knapsack: &spinach-knapsack stage: test @@ -110,6 +121,7 @@ stages: .spinach-knapsack-mysql: &spinach-knapsack-mysql <<: *spinach-knapsack <<: *use-mysql + <<: *only-master-and-ee-or-mysql # Prepare and merge knapsack tests -- cgit v1.2.1 From f6f6e2fb9bd2e7f76af6beeaa3e05d0b3de37f27 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 29 Mar 2017 00:25:03 +0800 Subject: Switch back to original repository --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c2c1668a761..1b0dd555b6e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -image: "registry.gitlab.com/godfat/gitlab-build-images:ruby-2.3.3-git-2.7-phantomjs-2.1-node-7.1-postgresql-9.6" +image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.3-git-2.7-phantomjs-2.1-node-7.1-postgresql-9.6" cache: key: "ruby-233" -- cgit v1.2.1 From 964814e99a30dd9fb0194e64cc01fee79bd027aa Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 29 Mar 2017 23:22:41 +0800 Subject: Only upload/download knapsack reports from pg --- .gitlab-ci.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1b0dd555b6e..a17438b2af2 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -77,7 +77,7 @@ stages: - JOB_NAME=( $CI_JOB_NAME ) - export CI_NODE_INDEX=${JOB_NAME[-2]} - export CI_NODE_TOTAL=${JOB_NAME[-1]} - - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json + - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_${JOB_NAME[1]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - export KNAPSACK_GENERATE_REPORT=true - cp ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} ${KNAPSACK_REPORT_PATH} - knapsack rspec "--color --format documentation" @@ -105,7 +105,7 @@ stages: - JOB_NAME=( $CI_JOB_NAME ) - export CI_NODE_INDEX=${JOB_NAME[-2]} - export CI_NODE_TOTAL=${JOB_NAME[-1]} - - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json + - export KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_${JOB_NAME[1]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json - export KNAPSACK_GENERATE_REPORT=true - cp ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} ${KNAPSACK_REPORT_PATH} - knapsack spinach "-r rerun" || retry '[[ -e tmp/spinach-rerun.txt ]] && bundle exec spinach -r rerun $(cat tmp/spinach-rerun.txt)' @@ -143,8 +143,8 @@ update-knapsack: <<: *dedicated-runner stage: post-test script: - - scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec_node_*.json - - scripts/merge-reports ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/spinach_node_*.json + - scripts/merge-reports ${KNAPSACK_RSPEC_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/rspec_pg_node_*.json + - scripts/merge-reports ${KNAPSACK_SPINACH_SUITE_REPORT_PATH} knapsack/${CI_PROJECT_NAME}/spinach_pg_node_*.json - '[[ -z ${KNAPSACK_S3_BUCKET} ]] || scripts/sync-reports put $KNAPSACK_S3_BUCKET $KNAPSACK_RSPEC_SUITE_REPORT_PATH $KNAPSACK_SPINACH_SUITE_REPORT_PATH' - rm -f knapsack/${CI_PROJECT_NAME}/*_node_*.json only: -- cgit v1.2.1 From c6701fef0e2082e1adae15adb97c75311115617f Mon Sep 17 00:00:00 2001 From: Fabio Huser Date: Mon, 27 Mar 2017 22:18:09 +0200 Subject: fix(subgroups): add verification of group creation capability to subgroup UI --- app/models/user.rb | 4 ++++ app/views/groups/subgroups.html.haml | 2 +- spec/models/user_spec.rb | 22 ++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index cbd741f96ed..bed2f0cae53 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -570,6 +570,10 @@ class User < ActiveRecord::Base can?(:create_group) end + def can_create_subgroup?(group) + can?(:create_group) && can?(:admin_group, group) + end + def can_select_namespace? several_namespaces? || admin end diff --git a/app/views/groups/subgroups.html.haml b/app/views/groups/subgroups.html.haml index be809083139..3342ba118ef 100644 --- a/app/views/groups/subgroups.html.haml +++ b/app/views/groups/subgroups.html.haml @@ -9,7 +9,7 @@ .nav-controls = form_tag request.path, method: :get do |f| = search_field_tag :filter_groups, params[:filter_groups], placeholder: 'Filter by name', class: 'form-control', spellcheck: false - - if can? current_user, :admin_group, @group + - if current_user.can_create_subgroup? @group = link_to new_group_path(parent_id: @group.id), class: 'btn btn-new pull-right' do New Subgroup diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a9e37be1157..575b43c3d88 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -464,6 +464,28 @@ describe User, models: true do it { expect(@user2.several_namespaces?).to be_truthy } end + describe 'subgroups' do + let(:group) { create :group } + + it 'allows if owner' do + user = create :user + group.add_user(user, GroupMember::OWNER) + expect(user.can_create_subgroup?(group)).to be_truthy + end + + it 'disallows if missing right' do + user = create(:user, can_create_group: false) + group.add_user(user, GroupMember::MASTER) + expect(user.can_create_subgroup?(group)).to be_falsey + end + + it 'disallows if developer' do + user = create :user + group.add_user(user, GroupMember::DEVELOPER) + expect(user.can_create_subgroup?(group)).to be_falsey + end + end + describe 'namespaced' do before do @user = create :user -- cgit v1.2.1 From c67271cd78f3df1090996feabd08bb713c71f7ca Mon Sep 17 00:00:00 2001 From: Guillaume Date: Sun, 5 Mar 2017 22:08:15 +0100 Subject: Database SSL support for backup script. --- .../unreleased/1440-db-backup-ssl-support.yml | 4 ++++ lib/backup/database.rb | 26 +++++++++++++++++----- 2 files changed, 25 insertions(+), 5 deletions(-) create mode 100644 changelogs/unreleased/1440-db-backup-ssl-support.yml diff --git a/changelogs/unreleased/1440-db-backup-ssl-support.yml b/changelogs/unreleased/1440-db-backup-ssl-support.yml new file mode 100644 index 00000000000..c78bb4fd351 --- /dev/null +++ b/changelogs/unreleased/1440-db-backup-ssl-support.yml @@ -0,0 +1,4 @@ +--- +title: Database SSL support for backup script. +merge_request: 9715 +author: Guillaume Simon diff --git a/lib/backup/database.rb b/lib/backup/database.rb index 4016ac76348..d97e5d98229 100644 --- a/lib/backup/database.rb +++ b/lib/backup/database.rb @@ -80,16 +80,32 @@ module Backup 'port' => '--port', 'socket' => '--socket', 'username' => '--user', - 'encoding' => '--default-character-set' + 'encoding' => '--default-character-set', + # SSL + 'sslkey' => '--ssl-key', + 'sslcert' => '--ssl-cert', + 'sslca' => '--ssl-ca', + 'sslcapath' => '--ssl-capath', + 'sslcipher' => '--ssl-cipher' } args.map { |opt, arg| "#{arg}=#{config[opt]}" if config[opt] }.compact end def pg_env - ENV['PGUSER'] = config["username"] if config["username"] - ENV['PGHOST'] = config["host"] if config["host"] - ENV['PGPORT'] = config["port"].to_s if config["port"] - ENV['PGPASSWORD'] = config["password"].to_s if config["password"] + args = { + 'username' => 'PGUSER', + 'host' => 'PGHOST', + 'port' => 'PGPORT', + 'password' => 'PGPASSWORD', + # SSL + 'sslmode' => 'PGSSLMODE', + 'sslkey' => 'PGSSLKEY', + 'sslcert' => 'PGSSLCERT', + 'sslrootcert' => 'PGSSLROOTCERT', + 'sslcrl' => 'PGSSLCRL', + 'sslcompression' => 'PGSSLCOMPRESSION' + } + args.each { |opt, arg| ENV[arg] = config[opt].to_s if config[opt] } end def report_success(success) -- cgit v1.2.1 From b742ff96fc9e844c9dc17b6b7a7f62b3497594ba Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Fri, 7 Apr 2017 14:26:56 +0200 Subject: Improve gitaly_address error message Closes gitaly#174 --- lib/gitlab/gitaly_client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index bcdf1b1faa8..c69676a1dac 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -15,7 +15,7 @@ module Gitlab end unless URI(address).scheme.in?(%w(tcp unix)) - raise "Unsupported Gitaly address: #{address.inspect}" + raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix'" end @addresses[name] = address -- cgit v1.2.1 From 8559a900641806b54a78737679060099e34b2baa Mon Sep 17 00:00:00 2001 From: andrew brown Date: Sun, 9 Apr 2017 12:41:23 -0700 Subject: Use the hashie-forbideen_attributes gem This gem prevents Mash from responding to :permitted?, disabling mass assignment protection for the Grape API --- Gemfile | 3 +++ Gemfile.lock | 5 ++++- changelogs/unreleased/use-hashie-forbidden_attributes.yml | 4 ++++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/use-hashie-forbidden_attributes.yml diff --git a/Gemfile b/Gemfile index d4b2ade4243..ad8db206da6 100644 --- a/Gemfile +++ b/Gemfile @@ -73,6 +73,9 @@ gem 'grape', '~> 0.19.0' gem 'grape-entity', '~> 0.6.0' gem 'rack-cors', '~> 0.4.0', require: 'rack/cors' +# Disable strong_params so that Mash does not respond to :permitted? +gem 'hashie-forbidden_attributes' + # Pagination gem 'kaminari', '~> 0.17.0' diff --git a/Gemfile.lock b/Gemfile.lock index d7e3f7343d0..bb91db1e805 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -346,6 +346,8 @@ GEM tilt hashdiff (0.3.2) hashie (3.5.5) + hashie-forbidden_attributes (0.1.1) + hashie (>= 3.0) health_check (2.6.0) rails (>= 4.0) hipchat (1.5.2) @@ -915,6 +917,7 @@ DEPENDENCIES grape-entity (~> 0.6.0) haml_lint (~> 0.21.0) hamlit (~> 2.6.1) + hashie-forbidden_attributes health_check (~> 2.6.0) hipchat (~> 1.5.0) html-pipeline (~> 1.11.0) @@ -1035,4 +1038,4 @@ DEPENDENCIES wikicloth (= 0.8.1) BUNDLED WITH - 1.14.5 + 1.14.6 diff --git a/changelogs/unreleased/use-hashie-forbidden_attributes.yml b/changelogs/unreleased/use-hashie-forbidden_attributes.yml new file mode 100644 index 00000000000..4f429b03a0d --- /dev/null +++ b/changelogs/unreleased/use-hashie-forbidden_attributes.yml @@ -0,0 +1,4 @@ +--- +title: Add hashie-forbidden_attributes gem +merge_request: 10579 +author: Andy Brown -- cgit v1.2.1 From a9945a6500351331426226efe80631f81cb832aa Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 10 Apr 2017 18:41:35 +0800 Subject: Replace on host rather than socket, feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10156#note_26812377 --- scripts/prepare_build.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index f170743aea3..b8d8cc2851a 100755 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -14,7 +14,7 @@ if [ -f /.dockerenv ] || [ -f ./dockerinit ]; then else # Assume it's mysql sed -i 's/username:.*/username: root/g' config/database.yml sed -i 's/password:.*/password:/g' config/database.yml - sed -i 's/# socket:.*/host: mysql/g' config/database.yml + sed -i 's/# host:.*/host: mysql/g' config/database.yml fi cp config/resque.yml.example config/resque.yml -- cgit v1.2.1 From f1e2387e2851009cf2091ab22b3d6a70a27dfa1e Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 10 Apr 2017 19:09:02 +0800 Subject: Use GITLAB_DATABASE: $CI_JOB_NAME[1] so that we reduce variables definitions. Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10156#note_26811996 --- .gitlab-ci.yml | 46 +++++++++++++++++----------------------------- scripts/prepare_build.sh | 4 ++++ 2 files changed, 21 insertions(+), 29 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a900825b3de..ee590e1d604 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,6 +7,7 @@ cache: variables: MYSQL_ALLOW_EMPTY_PASSWORD: "1" + GITLAB_DATABASE: $CI_JOB_NAME[1] RAILS_ENV: "test" NODE_ENV: "test" SIMPLECOV: "true" @@ -51,15 +52,11 @@ stages: services: - postgres:latest - redis:alpine - variables: - GITLAB_DATABASE: "postgresql" .use-mysql: &use-mysql services: - mysql:latest - redis:alpine - variables: - GITLAB_DATABASE: "mysql" .only-master-and-ee-or-mysql: &only-master-and-ee-or-mysql only: @@ -288,11 +285,11 @@ rake ee_compat_check: script: - bundle exec rake db:migrate:reset -pg rake db:migrate:reset: +rake pg db:migrate:reset: <<: *db-migrate-reset <<: *use-pg -mysql rake db:migrate:reset: +rake mysql db:migrate:reset: <<: *db-migrate-reset <<: *use-mysql @@ -303,19 +300,14 @@ mysql rake db:migrate:reset: - bundle exec rake db:rollback STEP=120 - bundle exec rake db:migrate -pg rake db:rollback: +rake pg db:rollback: <<: *db-rollback <<: *use-pg -mysql rake db:rollback: +rake mysql db:rollback: <<: *db-rollback <<: *use-mysql -.db-seed_fu-variables: &db-seed_fu-variables - SIZE: "1" - SETUP_DB: "false" - RAILS_ENV: "development" - .db-seed_fu: &db-seed_fu stage: test <<: *dedicated-runner @@ -323,25 +315,24 @@ mysql rake db:rollback: - git clone https://gitlab.com/gitlab-org/gitlab-test.git /home/git/repositories/gitlab-org/gitlab-test.git - bundle exec rake db:setup db:seed_fu + variables: + GITLAB_DATABASE: $CI_JOB_NAME[1] + SIZE: "1" + SETUP_DB: "false" + RAILS_ENV: "development" artifacts: when: on_failure expire_in: 1d paths: - log/development.log -pg rake db:seed_fu: +rake pg db:seed_fu: <<: *db-seed_fu <<: *use-pg - variables: - <<: *db-seed_fu-variables - GITLAB_DATABASE: "postgresql" -mysql rake db:seed_fu: +rake mysql db:seed_fu: <<: *db-seed_fu <<: *use-mysql - variables: - <<: *db-seed_fu-variables - GITLAB_DATABASE: "mysql" rake gitlab:assets:compile: stage: test @@ -444,20 +435,17 @@ bundler:audit: - bundle install --without production --jobs $(nproc) $FLAGS --retry=3 - source scripts/prepare_build.sh - bundle exec rake db:migrate + variables: + GITLAB_DATABASE: $CI_JOB_NAME[1] + SETUP_DB: "false" -pg migration paths: +migration pg paths: <<: *migration-paths <<: *use-pg - variables: - SETUP_DB: "false" - GITLAB_DATABASE: "postgresql" -mysql migration paths: +migration mysql paths: <<: *migration-paths <<: *use-mysql - variables: - SETUP_DB: "false" - GITLAB_DATABASE: "mysql" coverage: stage: post-test diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index b8d8cc2851a..2b041892eab 100755 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -6,6 +6,10 @@ export SETUP_DB=${SETUP_DB:-true} export GITLAB_DATABASE=${GITLAB_DATABASE:-postgresql} export USE_BUNDLE_INSTALL=${USE_BUNDLE_INSTALL:-true} +if [ "$GITLAB_DATABASE" = 'pg' ]; then + export GITLAB_DATABASE='postgresql' +fi + if [ -f /.dockerenv ] || [ -f ./dockerinit ]; then cp config/database.yml.$GITLAB_DATABASE config/database.yml -- cgit v1.2.1 From 76e7d2564d30a72604496acf8cd6fb1228905c0f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 10 Apr 2017 19:21:45 +0800 Subject: Set profiler only for postgresql not mysql Feedback: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10156#note_26812609 --- config/initializers/rspec_profiling.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/initializers/rspec_profiling.rb b/config/initializers/rspec_profiling.rb index 764c067c6f0..b909cc5b9a4 100644 --- a/config/initializers/rspec_profiling.rb +++ b/config/initializers/rspec_profiling.rb @@ -38,7 +38,7 @@ if Rails.env.test? end end - if ENV.has_key?('CI') + if ENV.has_key?('CI') && ENV['GITLAB_DATABASE'] == 'postgresql' RspecProfiling::VCS::Git.prepend(RspecProfilingExt::Git) RspecProfiling::Run.prepend(RspecProfilingExt::Run) end -- cgit v1.2.1 From e46f67a5123433f132da28770e4027542062fc81 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 10 Apr 2017 13:24:14 +0200 Subject: Use gRPC 1.2.2 Fixes LoadError after local compilation. https://github.com/grpc/grpc/issues/9998 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index d7e3f7343d0..965c888ca79 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -330,7 +330,7 @@ GEM grape-entity (0.6.0) activesupport multi_json (>= 1.3.2) - grpc (1.1.2) + grpc (1.2.2) google-protobuf (~> 3.1) googleauth (~> 0.5.1) haml (4.0.7) -- cgit v1.2.1 From 35bf7c7e47eb0ccd1f10fdfd19f9a85b426c184e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Wed, 22 Mar 2017 23:09:22 +0100 Subject: Firs pass at improving the testing documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [ci skip] Signed-off-by: Rémy Coutable --- doc/development/fe_guide/testing.md | 7 +- doc/development/testing.md | 218 +++++++++++++++++++++++++++++++----- 2 files changed, 194 insertions(+), 31 deletions(-) diff --git a/doc/development/fe_guide/testing.md b/doc/development/fe_guide/testing.md index 8d3513d3566..a8a01747c75 100644 --- a/doc/development/fe_guide/testing.md +++ b/doc/development/fe_guide/testing.md @@ -80,18 +80,17 @@ If an integration test depends on JavaScript to run correctly, you need to make sure the spec is configured to enable JavaScript when the tests are run. If you don't do this you'll see vague error messages from the spec runner. -To enable a JavaScript driver in an `rspec` test, add `js: true` to the +To enable a JavaScript driver in an `rspec` test, add `:js` to the individual spec or the context block containing multiple specs that need JavaScript enabled: ```ruby - # For one spec -it 'presents information about abuse report', js: true do +it 'presents information about abuse report', :js do # assertions... end -describe "Admin::AbuseReports", js: true do +describe "Admin::AbuseReports", :js do it 'presents information about abuse report' do # assertions... end diff --git a/doc/development/testing.md b/doc/development/testing.md index 5bc958f5a96..9dc75fd1337 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -9,6 +9,144 @@ this guide defines a rule that contradicts the thoughtbot guide, this guide takes precedence. Some guidelines may be repeated verbatim to stress their importance. +## Definitions + +### Unit tests + +Formal definition: https://en.wikipedia.org/wiki/Unit_testing + +These kind of tests ensure that a single unit of code (a method) works as expected (given an input, it has a predictable output). These tests should be isolated as much as possible (for instance model methods that don't do anything with the database shouldn't need a DB record). + +| Code path | Tests path | Testing engine | Notes | +| --------- | ---------- | -------------- | ----- | +| `app/finders/` | `spec/finders/` | RSpec | | +| `app/helpers/` | `spec/helpers/` | RSpec | | +| `app/migrations/` | `spec/migrations/` | RSpec | | +| `app/policies/` | `spec/policies/` | RSpec | | +| `app/presenters/` | `spec/presenters/` | RSpec | | +| `app/routing/` | `spec/routing/` | RSpec | | +| `app/serializers/` | `spec/serializers/` | RSpec | | +| `app/services/` | `spec/services/` | RSpec | | +| `app/tasks/` | `spec/tasks/` | RSpec | | +| `app/uploaders/` | `spec/uploaders/` | RSpec | | +| `app/views/` | `spec/views/` | RSpec | | +| `app/workers/` | `spec/workers/` | RSpec | | +| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. | + +### Integration tests + +Formal definition: https://en.wikipedia.org/wiki/Integration_testing + +These kind of tests ensure that individual parts of the application work well together, without the overhead of the actual app environment (i.e. the browser). These tests should assert at the request/response level: status code, headers, body. They're useful to test permissions, redirections, what view is rendered etc. + +| Code path | Tests path | Testing engine | Notes | +| --------- | ---------- | -------------- | ----- | +| `app/controllers/` | `spec/controllers/` | RSpec | | +| `lib/api/` | `spec/requests/api/` | RSpec | | +| `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | | +| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. | + +#### About controller tests + +In an ideal world, controllers should be thin. However, when this is not the +case, it's acceptable to write a system test without JavaScript instead of a +controller test. The reason is that testing a fat controller usually involves a +lot of stubbing, things like: + +```ruby +controller.instance_variable_set(:@user, user) +``` + +and use methods which are deprecated in Rails 5 ([#23768]). + +[#23768]: https://gitlab.com/gitlab-org/gitlab-ce/issues/23768 + +#### About Karma + +As you may have noticed, Karma is both in the Unit tests and the Integration +tests category. That's because Karma is a tool that provides an environment to +run JavaScript tests, so you can either run unit tests (e.g. test a single +JavaScript method), or integration tests (e.g. test a component that is composed +of multiple components). + +### System tests or Feature tests + +Formal definition: https://en.wikipedia.org/wiki/System_testing. + +These kind of tests ensure the application works as expected from a user point +of view (aka black-box testing). These tests should test a happy path for a +given page or set of pages, and a test case should be added for any regression +that couldn't have been caught at lower levels with better tests (i.e. if a +regression is found, regression tests should be added at the lowest-level +possible). + +| Tests path | Testing engine | Notes | +| ---------- | -------------- | ----- | +| `spec/features/` | [Capybara] + [RSpec] | If your spec has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. | +| `features/` | Spinach | Spinach tests are deprecated, [you shouldn't add new Spinach tests](#spinach-feature-tests). | + +[Capybara]: https://github.com/teamcapybara/capybara +[RSpec]: https://github.com/rspec/rspec-rails#feature-specs +[Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist +[RackTest]: https://github.com/teamcapybara/capybara#racktest + +#### Good practices + +- Create only the necessary records in the database +- Test a happy path and a less happy path but that's it +- Every other possible paths should be tested with Unit or Integration tests +- Test what's displayed on the page, not the internal of ActiveRecord models +- It's ok to look for DOM elements but don't abuse it since it makes the tests + more brittle + +If we're confident that the low-level components work well (and we should be if +we have enough Unit & Integration tests), we shouldn't need to duplicate their +thorough testing at the System test level. + +It's very easy to add tests, but a lot harder to remove or improve tests, so one +should take care of not introducing too many (slow and duplicated) specs. + +The reason why we should follow these good practices are as follows: + +- System tests are slow to run since they spin up the entire application stack + in a headless browser, and even slower when they integrate a JS driver +- With System tests run with a driver that supports JavaScript, the tests are + run in different thread than the application. This means it does not share a + database connection and your test will have to commit the transactions in + order for the running application to see the data (and vice-versa). In that + case we need to truncate the database after each spec instead of simply + rolling back a transaction (the faster strategy that's in use for other kind + of tests). This is slower than transactions, however, so we want to use + truncation only when necessary. + +## How to test at the correct level? + +As many things in life, deciding what to test at each level of testing is a +trade-off: + +- Unit tests are usually cheap, and you should consider them like the basement + of your house: you need them to be confident that your code is behaving + correctly. However if you run only unit tests without integration / system tests, you might miss the [big] [picture]! +- Integration tests are bit more expensive but don't abuse them. A feature test + is often better than an integration test that is stubbing a lot of internals. +- System tests are expensive (compared to unit tests), even more if they require + a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed) + section. + +Another way to see it is to think about the "cost of tests", this is well +explained [in this article][tests-cost] and the basic idea is that the cost of a +test includes: + +- The time it takes to write the test +- The time it takes to run the test every time the suite runs +- The time it takes to understand the test +- The time it takes to fix the test if it breaks and the underlying code is OK +- Maybe, the time it takes to change the code to make the code testable. + +[big]: https://twitter.com/timbray/status/822470746773409794 +[picture]: https://twitter.com/withzombies/status/829716565834752000 +[tests-cost]: https://medium.com/table-xi/high-cost-tests-and-high-value-tests-a86e27a54df#.2ulyh3a4e + ## Factories GitLab uses [factory_girl] as a test fixture replacement. @@ -117,11 +255,20 @@ it 'is overdue' do end ``` -### Test speed +### System / Features tests -GitLab has a massive test suite that, without parallelization, can take more -than an hour to run. It's important that we make an effort to write tests that -are accurate and effective _as well as_ fast. +- Feature specs should be named `ROLE_ACTION_spec.rb`, such as + `user_changes_password_spec.rb`. +- Use only one `feature` block per feature spec file. +- Use scenario titles that describe the success and failure paths. +- Avoid scenario titles that add no information, such as "successfully". +- Avoid scenario titles that repeat the feature title. + +## Test speed + +GitLab has a massive test suite that, without [parallelization], can take hours +to run. It's important that we make an effort to write tests that are accurate +and effective _as well as_ fast. Here are some things to keep in mind regarding test performance: @@ -132,38 +279,40 @@ Here are some things to keep in mind regarding test performance: - Use `create(:empty_project)` instead of `create(:project)` when you don't need the underlying Git repository. Filesystem operations are slow! - Don't mark a feature as requiring JavaScript (through `@javascript` in - Spinach or `js: true` in RSpec) unless it's _actually_ required for the test + Spinach or `:js` in RSpec) unless it's _actually_ required for the test to be valid. Headless browser testing is slow! -### Features / Integration +[parallelization]: #test-suite-parallelization-on-the-ci -GitLab uses [rspec-rails feature specs] to test features in a browser -environment. These are [capybara] specs running on the headless [poltergeist] -driver. +### Monitoring -- Feature specs live in `spec/features/` and should be named - `ROLE_ACTION_spec.rb`, such as `user_changes_password_spec.rb`. -- Use only one `feature` block per feature spec file. -- Use scenario titles that describe the success and failure paths. -- Avoid scenario titles that add no information, such as "successfully." -- Avoid scenario titles that repeat the feature title. +The GitLab test suite is [monitored] and a [public dashboard] is available for +everyone to see. Feel free to look at the slowest test files and try to improve +them. -[rspec-rails feature specs]: https://github.com/rspec/rspec-rails#feature-specs -[capybara]: https://github.com/teamcapybara/capybara -[poltergeist]: https://github.com/teampoltergeist/poltergeist +[monitored]: /development/performance.html#rspec-profiling +[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default -## Spinach (feature) tests +## Test suite parallelization on the CI -GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426) -for its feature/integration tests in September 2012. +Our current CI parallelization setup is as follows: -As of March 2016, we are [trying to avoid adding new Spinach -tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward, -opting for [RSpec feature](#features-integration) specs. +1. The `knapsack` job in the prepare stage that is supposed to ensure we have a `knapsack/rspec_report.json` file: + - The `knapsack/rspec_report.json` file is fetched from the cache with the + `knapsack` key, if it's not here we initialize the file with `{}`. +1. Each `rspec x y` job are run with `knapsack rspec` and should have an evenly + distributed share of tests: + - It works because the jobs have access to the `knapsack/rspec_report.json` + since the "artifacts from all previous stages are passed by default". [^1] + - the jobs set their own report path to `KNAPSACK_REPORT_PATH=knapsack/spinach_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` + - if knapsack is doing its job, test files that are run should be listed under + `Report specs`, not under `Leftover specs` +1. The `update-knapsack` job takes all the `knapsack/spinach_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` files from + the `rspec x y` jobs and merge them all together into a single + `knapsack/rspec_report.json` that is then cached with the `knapsack` key -Adding new Spinach scenarios is acceptable _only if_ the new scenario requires -no more than one new `step` definition. If more than that is required, the -test should be re-implemented using RSpec instead. +After that, the next pipeline will use the up-to-date +`knapsack/rspec_report.json` file. ## Testing Rake Tasks @@ -201,6 +350,21 @@ describe 'gitlab:shell rake tasks' do end ``` +## Spinach (feature) tests + +GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426) +for its feature/integration tests in September 2012. + +As of March 2016, we are [trying to avoid adding new Spinach +tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward, +opting for [RSpec feature](#features-integration) specs. + +Adding new Spinach scenarios is acceptable _only if_ the new scenario requires +no more than one new `step` definition. If more than that is required, the +test should be re-implemented using RSpec instead. + --- [Return to Development documentation](README.md) + +[^1]: /ci/yaml/README.html#dependencies -- cgit v1.2.1 From 91fb9f446fa8476f287657032003aa286c2606b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 7 Apr 2017 20:50:41 +0200 Subject: Improve testing documentation with Robert's feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- doc/development/fe_guide/testing.md | 27 ++++++++--------- doc/development/testing.md | 59 ++++++++++++++++++++++--------------- 2 files changed, 49 insertions(+), 37 deletions(-) diff --git a/doc/development/fe_guide/testing.md b/doc/development/fe_guide/testing.md index a8a01747c75..175873c9efa 100644 --- a/doc/development/fe_guide/testing.md +++ b/doc/development/fe_guide/testing.md @@ -87,16 +87,16 @@ JavaScript enabled: ```ruby # For one spec it 'presents information about abuse report', :js do - # assertions... + # assertions... end describe "Admin::AbuseReports", :js do - it 'presents information about abuse report' do - # assertions... - end - it 'shows buttons for adding to abuse report' do - # assertions... - end + it 'presents information about abuse report' do + # assertions... + end + it 'shows buttons for adding to abuse report' do + # assertions... + end end ``` @@ -112,13 +112,12 @@ file for the failing spec, add the `@javascript` flag above the Scenario: ``` @javascript Scenario: Developer can approve merge request - Given I am a "Shop" developer - And I visit project "Shop" merge requests page - And merge request 'Bug NS-04' must be approved - And I click link "Bug NS-04" - When I click link "Approve" - Then I should see approved merge request "Bug NS-04" - + Given I am a "Shop" developer + And I visit project "Shop" merge requests page + And merge request 'Bug NS-04' must be approved + And I click link "Bug NS-04" + When I click link "Approve" + Then I should see approved merge request "Bug NS-04" ``` [capybara]: http://teamcapybara.github.io/capybara/ diff --git a/doc/development/testing.md b/doc/development/testing.md index 9dc75fd1337..e096adcdf1c 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -15,7 +15,10 @@ importance. Formal definition: https://en.wikipedia.org/wiki/Unit_testing -These kind of tests ensure that a single unit of code (a method) works as expected (given an input, it has a predictable output). These tests should be isolated as much as possible (for instance model methods that don't do anything with the database shouldn't need a DB record). +These kind of tests ensure that a single unit of code (a method) works as +expected (given an input, it has a predictable output). These tests should be +isolated as much as possible (for example, model methods that don't do anything +with the database shouldn't need a DB record). | Code path | Tests path | Testing engine | Notes | | --------- | ---------- | -------------- | ----- | @@ -42,6 +45,7 @@ These kind of tests ensure that individual parts of the application work well to | Code path | Tests path | Testing engine | Notes | | --------- | ---------- | -------------- | ----- | | `app/controllers/` | `spec/controllers/` | RSpec | | +| `app/mailers/` | `spec/mailers/` | RSpec | | | `lib/api/` | `spec/requests/api/` | RSpec | | | `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | | | `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. | @@ -49,9 +53,9 @@ These kind of tests ensure that individual parts of the application work well to #### About controller tests In an ideal world, controllers should be thin. However, when this is not the -case, it's acceptable to write a system test without JavaScript instead of a -controller test. The reason is that testing a fat controller usually involves a -lot of stubbing, things like: +case, it's acceptable to write a system/feature test without JavaScript instead +of a controller test. The reason is that testing a fat controller usually +involves a lot of stubbing, things like: ```ruby controller.instance_variable_set(:@user, user) @@ -90,12 +94,15 @@ possible). [Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist [RackTest]: https://github.com/teamcapybara/capybara#racktest -#### Good practices +#### Best practices - Create only the necessary records in the database - Test a happy path and a less happy path but that's it -- Every other possible paths should be tested with Unit or Integration tests -- Test what's displayed on the page, not the internal of ActiveRecord models +- Every other possible path should be tested with Unit or Integration tests +- Test what's displayed on the page, not the internals of ActiveRecord models. + For instance, if you want to verify that a record was created, add + expectations that its attributes are displayed on the page, not that + `Model.count` increased by one. - It's ok to look for DOM elements but don't abuse it since it makes the tests more brittle @@ -106,12 +113,12 @@ thorough testing at the System test level. It's very easy to add tests, but a lot harder to remove or improve tests, so one should take care of not introducing too many (slow and duplicated) specs. -The reason why we should follow these good practices are as follows: +The reasons why we should follow these best practices are as follows: - System tests are slow to run since they spin up the entire application stack in a headless browser, and even slower when they integrate a JS driver -- With System tests run with a driver that supports JavaScript, the tests are - run in different thread than the application. This means it does not share a +- When system tests run with a JavaScript driver, the tests are run in a + different thread than the application. This means it does not share a database connection and your test will have to commit the transactions in order for the running application to see the data (and vice-versa). In that case we need to truncate the database after each spec instead of simply @@ -127,7 +134,7 @@ trade-off: - Unit tests are usually cheap, and you should consider them like the basement of your house: you need them to be confident that your code is behaving correctly. However if you run only unit tests without integration / system tests, you might miss the [big] [picture]! -- Integration tests are bit more expensive but don't abuse them. A feature test +- Integration tests are a bit more expensive, but don't abuse them. A feature test is often better than an integration test that is stubbing a lot of internals. - System tests are expensive (compared to unit tests), even more if they require a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed) @@ -255,7 +262,7 @@ it 'is overdue' do end ``` -### System / Features tests +### System / Feature tests - Feature specs should be named `ROLE_ACTION_spec.rb`, such as `user_changes_password_spec.rb`. @@ -297,22 +304,28 @@ them. Our current CI parallelization setup is as follows: -1. The `knapsack` job in the prepare stage that is supposed to ensure we have a `knapsack/rspec_report.json` file: - - The `knapsack/rspec_report.json` file is fetched from the cache with the - `knapsack` key, if it's not here we initialize the file with `{}`. +1. The `knapsack` job in the prepare stage that is supposed to ensure we have a + `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file: + - The `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file is fetched + from S3, if it's not here we initialize the file with `{}`. 1. Each `rspec x y` job are run with `knapsack rspec` and should have an evenly distributed share of tests: - - It works because the jobs have access to the `knapsack/rspec_report.json` - since the "artifacts from all previous stages are passed by default". [^1] - - the jobs set their own report path to `KNAPSACK_REPORT_PATH=knapsack/spinach_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` + - It works because the jobs have access to the + `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` since the "artifacts + from all previous stages are passed by default". [^1] + - the jobs set their own report path to + `KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`. - if knapsack is doing its job, test files that are run should be listed under - `Report specs`, not under `Leftover specs` -1. The `update-knapsack` job takes all the `knapsack/spinach_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` files from - the `rspec x y` jobs and merge them all together into a single - `knapsack/rspec_report.json` that is then cached with the `knapsack` key + `Report specs`, not under `Leftover specs`. +1. The `update-knapsack` job takes all the + `knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json` + files from the `rspec x y` jobs and merge them all together into a single + `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that is then + uploaded to S3. After that, the next pipeline will use the up-to-date -`knapsack/rspec_report.json` file. +`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. The same strategy +is used for Spinach tests as well. ## Testing Rake Tasks -- cgit v1.2.1 From 0aafb6abb89b4b0fc4c033b21ccbfd1082b56b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 10 Apr 2017 11:32:23 +0200 Subject: Document GitLab QA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- doc/development/testing.md | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/doc/development/testing.md b/doc/development/testing.md index e096adcdf1c..0d29697df9e 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -17,8 +17,9 @@ Formal definition: https://en.wikipedia.org/wiki/Unit_testing These kind of tests ensure that a single unit of code (a method) works as expected (given an input, it has a predictable output). These tests should be -isolated as much as possible (for example, model methods that don't do anything -with the database shouldn't need a DB record). +isolated as much as possible. For example, model methods that don't do anything +with the database shouldn't need a DB record. Classes that don't need database +records should use stubs/doubles as much as possible. | Code path | Tests path | Testing engine | Notes | | --------- | ---------- | -------------- | ----- | @@ -94,6 +95,29 @@ possible). [Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist [RackTest]: https://github.com/teamcapybara/capybara#racktest +### Black-box tests or End-to-end tests + +GitLab consists of [multiple pieces] such as [GitLab Shell], [GitLab Workhorse], +[Gitaly], [GitLab Pages], [GitLab Runner], and GitLab Rails. All theses pieces +are configured and packaged by [GitLab Omnibus]. + +[GitLab QA] is a tool that allows to test that all these pieces integrate well +together by building a Docker image for a given version of GitLab Rails and +running feature tests (i.e. using Capybara) against it. + +The actual test scenarios and steps are [part of GitLab Rails] so that they're +always in-sync with the codebase. + +[multiple pieces]: ./architecture.md#components +[GitLab Shell]: https://gitlab.com/gitlab-org/gitlab-shell +[GitLab Workhorse]: https://gitlab.com/gitlab-org/gitlab-workhorse +[Gitaly]: https://gitlab.com/gitlab-org/gitaly +[GitLab Pages]: https://gitlab.com/gitlab-org/gitlab-pages +[GitLab Runner]: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner +[GitLab Omnibus]: https://gitlab.com/gitlab-org/omnibus-gitlab +[GitLab QA]: https://gitlab.com/gitlab-org/gitlab-qa +[part of GitLab Rails]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa + #### Best practices - Create only the necessary records in the database @@ -133,7 +157,7 @@ trade-off: - Unit tests are usually cheap, and you should consider them like the basement of your house: you need them to be confident that your code is behaving - correctly. However if you run only unit tests without integration / system tests, you might miss the [big] [picture]! + correctly. However if you run only unit tests without integration / system tests, you might [miss] the [big] [picture]! - Integration tests are a bit more expensive, but don't abuse them. A feature test is often better than an integration test that is stubbing a lot of internals. - System tests are expensive (compared to unit tests), even more if they require @@ -150,6 +174,7 @@ test includes: - The time it takes to fix the test if it breaks and the underlying code is OK - Maybe, the time it takes to change the code to make the code testable. +[miss]: https://twitter.com/ThePracticalDev/status/850748070698651649 [big]: https://twitter.com/timbray/status/822470746773409794 [picture]: https://twitter.com/withzombies/status/829716565834752000 [tests-cost]: https://medium.com/table-xi/high-cost-tests-and-high-value-tests-a86e27a54df#.2ulyh3a4e -- cgit v1.2.1 From 2b606a3a2eb408c4c66a959f7efdfeeb76bb18e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 10 Apr 2017 15:23:26 +0200 Subject: Re-organize testing doc, and add RSpec structure doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- doc/development/fe_guide/testing.md | 11 +- doc/development/testing.md | 243 +++++++++++++++++++++++------------- 2 files changed, 165 insertions(+), 89 deletions(-) diff --git a/doc/development/fe_guide/testing.md b/doc/development/fe_guide/testing.md index 175873c9efa..a4631fd0073 100644 --- a/doc/development/fe_guide/testing.md +++ b/doc/development/fe_guide/testing.md @@ -13,10 +13,19 @@ for more information on general testing practices at GitLab. ## Karma test suite GitLab uses the [Karma][karma] test runner with [Jasmine][jasmine] as its test -framework for our JavaScript unit tests. For tests that rely on DOM +framework for our JavaScript unit tests. For tests that rely on DOM manipulation we use fixtures which are pre-compiled from HAML source files and served during testing by the [jasmine-jquery][jasmine-jquery] plugin. +JavaScript tests live in `spec/javascripts/`, matching the folder structure +of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js` +has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file. + +Keep in mind that in a CI environment, these tests are run in a headless +browser and you will not have access to certain APIs, such as +[`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification), +which will have to be stubbed. + ### Running frontend tests `rake karma` runs the frontend-only (JavaScript) tests. diff --git a/doc/development/testing.md b/doc/development/testing.md index 0d29697df9e..e530b1c07bd 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -179,52 +179,9 @@ test includes: [picture]: https://twitter.com/withzombies/status/829716565834752000 [tests-cost]: https://medium.com/table-xi/high-cost-tests-and-high-value-tests-a86e27a54df#.2ulyh3a4e -## Factories +## Frontend testing -GitLab uses [factory_girl] as a test fixture replacement. - -- Factory definitions live in `spec/factories/`, named using the pluralization - of their corresponding model (`User` factories are defined in `users.rb`). -- There should be only one top-level factory definition per file. -- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and - should) call `create(...)` instead of `FactoryGirl.create(...)`. -- Make use of [traits] to clean up definitions and usages. -- When defining a factory, don't define attributes that are not required for the - resulting record to pass validation. -- When instantiating from a factory, don't supply attributes that aren't - required by the test. -- Factories don't have to be limited to `ActiveRecord` objects. - [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). - -[factory_girl]: https://github.com/thoughtbot/factory_girl -[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits - -## JavaScript - -GitLab uses [Karma] to run its [Jasmine] JavaScript specs. They can be run on -the command line via `bundle exec karma`. - -- JavaScript tests live in `spec/javascripts/`, matching the folder structure - of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js` - has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file. -- Haml fixtures required for JavaScript tests live in - `spec/javascripts/fixtures`. They should contain the bare minimum amount of - markup necessary for the test. - - > **Warning:** Keep in mind that a Rails view may change and - invalidate your test, but everything will still pass because your fixture - doesn't reflect the latest view. Because of this we encourage you to - generate fixtures from actual rails views whenever possible. - -- Keep in mind that in a CI environment, these tests are run in a headless - browser and you will not have access to certain APIs, such as - [`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification), - which will have to be stubbed. - -[Karma]: https://github.com/karma-runner/karma -[Jasmine]: https://github.com/jasmine/jasmine - -For more information, see the [frontend testing guide](fe_guide/testing.md). +Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md). ## RSpec @@ -296,6 +253,152 @@ end - Avoid scenario titles that add no information, such as "successfully". - Avoid scenario titles that repeat the feature title. +### Matchers + +Custom matchers should be created to clarify the intent and/or hide the +complexity of RSpec expectations.They should be placed under +`spec/support/matchers/`. Matchers can be placed in subfolder if they apply to +a certain type of specs only (e.g. features, requests etc.) but shouldn't be if +they apply to multiple type of specs. + +### Shared contexts + +All shared contexts should be be placed under `spec/support/shared_contexts/`. +Shared contexts can be placed in subfolder if they apply to a certain type of +specs only (e.g. features, requests etc.) but shouldn't be if they apply to +multiple type of specs. + +Each file should include only one context and have a descriptive name, e.g. +`spec/support/shared_contexts/controllers/githubish_import_controller_shared_context.rb`. + +### Shared examples + +All shared examples should be be placed under `spec/support/shared_examples/`. +Shared examples can be placed in subfolder if they apply to a certain type of +specs only (e.g. features, requests etc.) but shouldn't be if they apply to +multiple type of specs. + +Each file should include only one context and have a descriptive name, e.g. +`spec/support/shared_examples/controllers/githubish_import_controller_shared_example.rb`. + +### Helpers + +Helpers are usually modules that provide some methods to hide the complexity of +specific RSpec examples. You can define helpers in RSpec files if they're not +intended to be shared with other specs. Otherwise, they should be be placed +under `spec/support/helpers/`. Helpers can be placed in subfolder if they apply +to a certain type of specs only (e.g. features, requests etc.) but shouldn't be +if they apply to multiple type of specs. + +Helpers should follow the Rails naming / namespacing convention. For instance +`spec/support/helpers/cycle_analytics_helpers.rb` should define: + +```ruby +module Spec + module Support + module Helpers + module CycleAnalyticsHelpers + def create_commit_referencing_issue(issue, branch_name: random_git_name) + project.repository.add_branch(user, branch_name, 'master') + create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name) + end + end + end + end +end +``` + +Helpers should not change the RSpec config. For instance, the helpers module +described above should not include: + +```ruby +RSpec.configure do |config| + config.include Spec::Support::Helpers::CycleAnalyticsHelpers +end +``` + +### Factories + +GitLab uses [factory_girl] as a test fixture replacement. + +- Factory definitions live in `spec/factories/`, named using the pluralization + of their corresponding model (`User` factories are defined in `users.rb`). +- There should be only one top-level factory definition per file. +- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and + should) call `create(...)` instead of `FactoryGirl.create(...)`. +- Make use of [traits] to clean up definitions and usages. +- When defining a factory, don't define attributes that are not required for the + resulting record to pass validation. +- When instantiating from a factory, don't supply attributes that aren't + required by the test. +- Factories don't have to be limited to `ActiveRecord` objects. + [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). + +[factory_girl]: https://github.com/thoughtbot/factory_girl +[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits + +### Fixtures + +All fixtures should be be placed under `spec/fixtures/`. + +### Config + +RSpec config files are files that change the RSpec config (i.e. +`RSpec.configure do |config|` blocks). They should be placed under +`spec/support/config/`. + +Each file should be related to a specific domain, e.g. +`spec/support/config/capybara.rb`, `spec/support/config/carrierwave.rb`, etc. + +Helpers can be included in the `spec/support/config/rspec.rb` file. If a +helpers module applies only to a certain kind of specs, it should add modifiers +to the `config.include` call. For instance if +`spec/support/helpers/cycle_analytics_helpers.rb` applies to `:lib` and +`type: :model` specs only, you would write the following: + +```ruby +RSpec.configure do |config| + config.include Spec::Support::Helpers::CycleAnalyticsHelpers, :lib + config.include Spec::Support::Helpers::CycleAnalyticsHelpers, type: :model +end +``` + +## Testing Rake Tasks + +To make testing Rake tasks a little easier, there is a helper that can be included +in lieu of the standard Spec helper. Instead of `require 'spec_helper'`, use +`require 'rake_helper'`. The helper includes `spec_helper` for you, and configures +a few other things to make testing Rake tasks easier. + +At a minimum, requiring the Rake helper will redirect `stdout`, include the +runtime task helpers, and include the `RakeHelpers` Spec support module. + +The `RakeHelpers` module exposes a `run_rake_task()` method to make +executing tasks simple. See `spec/support/rake_helpers.rb` for all available +methods. + +Example: + +```ruby +require 'rake_helper' + +describe 'gitlab:shell rake tasks' do + before do + Rake.application.rake_require 'tasks/gitlab/shell' + + stub_warn_user_is_not_gitlab + end + + describe 'install task' do + it 'invokes create_hooks task' do + expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke) + + run_rake_task('gitlab:shell:install') + end + end +end +``` + ## Test speed GitLab has a massive test suite that, without [parallelization], can take hours @@ -316,16 +419,7 @@ Here are some things to keep in mind regarding test performance: [parallelization]: #test-suite-parallelization-on-the-ci -### Monitoring - -The GitLab test suite is [monitored] and a [public dashboard] is available for -everyone to see. Feel free to look at the slowest test files and try to improve -them. - -[monitored]: /development/performance.html#rspec-profiling -[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default - -## Test suite parallelization on the CI +### Test suite parallelization on the CI Our current CI parallelization setup is as follows: @@ -352,41 +446,14 @@ After that, the next pipeline will use the up-to-date `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. The same strategy is used for Spinach tests as well. -## Testing Rake Tasks - -To make testing Rake tasks a little easier, there is a helper that can be included -in lieu of the standard Spec helper. Instead of `require 'spec_helper'`, use -`require 'rake_helper'`. The helper includes `spec_helper` for you, and configures -a few other things to make testing Rake tasks easier. - -At a minimum, requiring the Rake helper will redirect `stdout`, include the -runtime task helpers, and include the `RakeHelpers` Spec support module. - -The `RakeHelpers` module exposes a `run_rake_task()` method to make -executing tasks simple. See `spec/support/rake_helpers.rb` for all available -methods. - -Example: - -```ruby -require 'rake_helper' - -describe 'gitlab:shell rake tasks' do - before do - Rake.application.rake_require 'tasks/gitlab/shell' - - stub_warn_user_is_not_gitlab - end +### Monitoring - describe 'install task' do - it 'invokes create_hooks task' do - expect(Rake::Task['gitlab:shell:create_hooks']).to receive(:invoke) +The GitLab test suite is [monitored] and a [public dashboard] is available for +everyone to see. Feel free to look at the slowest test files and try to improve +them. - run_rake_task('gitlab:shell:install') - end - end -end -``` +[monitored]: /development/performance.html#rspec-profiling +[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default ## Spinach (feature) tests -- cgit v1.2.1 From c7011c88901c7fa8972f02444bd5e3feef698ca4 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Mon, 10 Apr 2017 10:58:41 -0500 Subject: Use avatars instead of icons in activity view --- app/assets/stylesheets/pages/events.scss | 13 +++++++----- app/views/events/event/_common.html.haml | 24 +++++++++++++---------- app/views/events/event/_created_project.html.haml | 8 ++++++-- app/views/events/event/_note.html.haml | 8 ++++++-- app/views/events/event/_push.html.haml | 14 ++++++++----- 5 files changed, 43 insertions(+), 24 deletions(-) diff --git a/app/assets/stylesheets/pages/events.scss b/app/assets/stylesheets/pages/events.scss index 79da490675a..6b5cf394d00 100644 --- a/app/assets/stylesheets/pages/events.scss +++ b/app/assets/stylesheets/pages/events.scss @@ -10,10 +10,14 @@ position: relative; &.event-inline { - .profile-icon { + .system-note-image { top: 20px; } + .user-avatar { + top: 14px; + } + .event-title, .event-item-timestamp { line-height: 40px; @@ -24,7 +28,7 @@ color: $gl-text-color; } - .profile-icon { + .system-note-image { position: absolute; left: 0; top: 14px; @@ -128,8 +132,7 @@ li { &.commit { background: transparent; - padding: 3px; - padding-left: 0; + padding: 0; border: none; .commit-row-title { @@ -183,7 +186,7 @@ max-width: 100%; } - .profile-icon { + .system-note-image { display: none; } diff --git a/app/views/events/event/_common.html.haml b/app/views/events/event/_common.html.haml index af97e9588a5..e6bb05e975e 100644 --- a/app/views/events/event/_common.html.haml +++ b/app/views/events/event/_common.html.haml @@ -1,13 +1,17 @@ -- if event.target - - if event.action_name == "opened" - .profile-icon.open-icon - = custom_icon("icon_status_open") - - elsif event.action_name == "closed" - .profile-icon.closed-icon - = custom_icon("icon_status_closed") - - else - .profile-icon.fork-icon - = custom_icon("icon_code_fork") +- if current_path?('dashboard#activity') + .system-note-image.user-avatar + = author_avatar(event, size: 32) +- else + - if event.target + - if event.action_name == "opened" + .system-note-image.open-icon + = custom_icon("icon_status_open") + - elsif event.action_name == "closed" + .system-note-image.closed-icon + = custom_icon("icon_status_closed") + - else + .system-note-image.fork-icon + = custom_icon("icon_code_fork") .event-title %span.author_name= link_to_author event diff --git a/app/views/events/event/_created_project.html.haml b/app/views/events/event/_created_project.html.haml index fee85c94277..259533bd400 100644 --- a/app/views/events/event/_created_project.html.haml +++ b/app/views/events/event/_created_project.html.haml @@ -1,5 +1,9 @@ -.profile-icon.open-icon - = custom_icon("icon_status_open") +- if current_path?('dashboard#activity') + .system-note-image.user-avatar + = author_avatar(event, size: 32) +- else + .system-note-image.open-icon + = custom_icon("icon_status_open") .event-title %span.author_name= link_to_author event diff --git a/app/views/events/event/_note.html.haml b/app/views/events/event/_note.html.haml index 83709f5e4d0..1785fe6ab16 100644 --- a/app/views/events/event/_note.html.haml +++ b/app/views/events/event/_note.html.haml @@ -1,5 +1,9 @@ -.profile-icon - = custom_icon("icon_comment_o") +- if current_path?('dashboard#activity') + .system-note-image.user-avatar + = author_avatar(event, size: 32) +- else + .system-note-image + = custom_icon("icon_comment_o") .event-title %span.author_name= link_to_author event diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index efdc8764acf..8acee9c1da0 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -1,10 +1,14 @@ - project = event.project -.profile-icon - - if event.action_name == "deleted" - = custom_icon("trash_o") - - else - = custom_icon("icon_commit") +- if current_path?('dashboard#activity') + .system-note-image.user-avatar + = author_avatar(event, size: 32) +- else + .system-note-image + - if event.action_name == "deleted" + = custom_icon("trash_o") + - else + = custom_icon("icon_commit") .event-title %span.author_name= link_to_author event -- cgit v1.2.1 From c78f03ae6ef446e9862e185eba2dc3116b04fdb5 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 10 Apr 2017 21:59:53 +0800 Subject: We cannot use array in yaml variables --- .gitlab-ci.yml | 1 - scripts/prepare_build.sh | 8 ++++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ee590e1d604..d08fbd58fe1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -7,7 +7,6 @@ cache: variables: MYSQL_ALLOW_EMPTY_PASSWORD: "1" - GITLAB_DATABASE: $CI_JOB_NAME[1] RAILS_ENV: "test" NODE_ENV: "test" SIMPLECOV: "true" diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index 2b041892eab..48b046116e4 100755 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -2,11 +2,15 @@ . scripts/utils.sh +echo $CI_JOB_NAME +job_name=$(echo $CI_JOB_NAME | cut -f2 -d' ') +echo $job_name + export SETUP_DB=${SETUP_DB:-true} -export GITLAB_DATABASE=${GITLAB_DATABASE:-postgresql} +export GITLAB_DATABASE=${GITLAB_DATABASE:-$job_name} export USE_BUNDLE_INSTALL=${USE_BUNDLE_INSTALL:-true} -if [ "$GITLAB_DATABASE" = 'pg' ]; then +if [ "$GITLAB_DATABASE" != 'mysql' ]; then export GITLAB_DATABASE='postgresql' fi -- cgit v1.2.1 From 1b14a8d001941895d79fcbe807bef2f052b4289f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 11 Apr 2017 01:27:36 +0800 Subject: Add # host for development and test as well --- config/database.yml.mysql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/database.yml.mysql b/config/database.yml.mysql index a33e40e8eb3..db1b712d3bc 100644 --- a/config/database.yml.mysql +++ b/config/database.yml.mysql @@ -25,6 +25,7 @@ development: pool: 5 username: root password: "secure password" + # host: localhost # socket: /tmp/mysql.sock # Warning: The database defined as "test" will be erased and @@ -39,4 +40,5 @@ test: &test pool: 5 username: root password: + # host: localhost # socket: /tmp/mysql.sock -- cgit v1.2.1 From 5e6d09edba4c782237544ac46edc320a27bcd98f Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 11 Apr 2017 01:56:30 +0800 Subject: Just set GITLAB_DATABASE in the script --- .gitlab-ci.yml | 2 -- scripts/prepare_build.sh | 6 +----- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index d08fbd58fe1..cbe7d02389a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -315,7 +315,6 @@ rake mysql db:rollback: /home/git/repositories/gitlab-org/gitlab-test.git - bundle exec rake db:setup db:seed_fu variables: - GITLAB_DATABASE: $CI_JOB_NAME[1] SIZE: "1" SETUP_DB: "false" RAILS_ENV: "development" @@ -435,7 +434,6 @@ bundler:audit: - source scripts/prepare_build.sh - bundle exec rake db:migrate variables: - GITLAB_DATABASE: $CI_JOB_NAME[1] SETUP_DB: "false" migration pg paths: diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index 48b046116e4..81fdf753c4e 100755 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -2,12 +2,8 @@ . scripts/utils.sh -echo $CI_JOB_NAME -job_name=$(echo $CI_JOB_NAME | cut -f2 -d' ') -echo $job_name - export SETUP_DB=${SETUP_DB:-true} -export GITLAB_DATABASE=${GITLAB_DATABASE:-$job_name} +export GITLAB_DATABASE=$(echo $CI_JOB_NAME | cut -f2 -d' ') export USE_BUNDLE_INSTALL=${USE_BUNDLE_INSTALL:-true} if [ "$GITLAB_DATABASE" != 'mysql' ]; then -- cgit v1.2.1 From c31c70b2fa01acef8842e68a3799ba72d0b3300a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 11 Apr 2017 02:32:20 +0800 Subject: Move variables back to where they are --- .gitlab-ci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cbe7d02389a..31db88a19fa 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -310,14 +310,14 @@ rake mysql db:rollback: .db-seed_fu: &db-seed_fu stage: test <<: *dedicated-runner - script: - - git clone https://gitlab.com/gitlab-org/gitlab-test.git - /home/git/repositories/gitlab-org/gitlab-test.git - - bundle exec rake db:setup db:seed_fu variables: SIZE: "1" SETUP_DB: "false" RAILS_ENV: "development" + script: + - git clone https://gitlab.com/gitlab-org/gitlab-test.git + /home/git/repositories/gitlab-org/gitlab-test.git + - bundle exec rake db:setup db:seed_fu artifacts: when: on_failure expire_in: 1d @@ -417,6 +417,8 @@ bundler:audit: .migration-paths: &migration-paths stage: test <<: *dedicated-runner + variables: + SETUP_DB: "false" only: - master@gitlab-org/gitlab-ce - master@gitlab-org/gitlab-ee @@ -433,8 +435,6 @@ bundler:audit: - bundle install --without production --jobs $(nproc) $FLAGS --retry=3 - source scripts/prepare_build.sh - bundle exec rake db:migrate - variables: - SETUP_DB: "false" migration pg paths: <<: *migration-paths -- cgit v1.2.1 From 400c328fb3e63ae19f8822a2f02744060f73b7ad Mon Sep 17 00:00:00 2001 From: Victor Wu Date: Mon, 10 Apr 2017 19:20:14 +0000 Subject: high-priority-features-in-patch-releases --- PROCESS.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/PROCESS.md b/PROCESS.md index cfa841dc13d..fa9e375b226 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -136,9 +136,11 @@ against the potential negative impact (things breaking without enough time to comfortably find and fix them before the release on the 22nd). When in doubt, we err on the side of _not_ cherry-picking. -For example, it is likely that an exception will be made for a trivial 1-5 line performance improvement +For example, it is likely that an exception will be made for a trivial 1-5 line performance improvement, (e.g. adding a database index or adding `includes` to a query), but not for a new feature, no matter how relatively small or thoroughly tested. +Another exception example is a medium-risk feature such as a navigation change in response to significant negative community feedback from a previous release. + During the feature freeze all merge requests that are meant to go into the upcoming release should have the correct milestone assigned _and_ have the label ~"Pick into Stable" set, so that release managers can find and pick them. -- cgit v1.2.1 From 530d17f1e207fe60d0e854fa57fb02f7c0ea4ca6 Mon Sep 17 00:00:00 2001 From: Victor Wu Date: Mon, 10 Apr 2017 19:23:35 +0000 Subject: remove stray comma [ci skip] --- PROCESS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PROCESS.md b/PROCESS.md index fa9e375b226..3efbb8f83ab 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -136,7 +136,7 @@ against the potential negative impact (things breaking without enough time to comfortably find and fix them before the release on the 22nd). When in doubt, we err on the side of _not_ cherry-picking. -For example, it is likely that an exception will be made for a trivial 1-5 line performance improvement, +For example, it is likely that an exception will be made for a trivial 1-5 line performance improvement (e.g. adding a database index or adding `includes` to a query), but not for a new feature, no matter how relatively small or thoroughly tested. Another exception example is a medium-risk feature such as a navigation change in response to significant negative community feedback from a previous release. -- cgit v1.2.1 From 4f839b4e63045f8d705bb02d95aa9c1ee88b6fb0 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Mon, 10 Apr 2017 22:13:45 +0200 Subject: Replace pipelines image in docs [ci skip] --- doc/ci/img/pipelines.png | Bin 7516 -> 6298 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/doc/ci/img/pipelines.png b/doc/ci/img/pipelines.png index 5937e9d99c8..a604fcb2587 100644 Binary files a/doc/ci/img/pipelines.png and b/doc/ci/img/pipelines.png differ -- cgit v1.2.1 From 6c4d611772ad35b4559f1a0c8812128ffa5ed091 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 10 Apr 2017 18:52:46 -0500 Subject: Remove IIFEs in boards_bundle.js --- app/assets/javascripts/boards/components/board.js | 168 +++++++------ .../javascripts/boards/components/board_delete.js | 28 +-- .../javascripts/boards/components/board_sidebar.js | 112 +++++---- .../boards/components/issue_card_inner.js | 246 +++++++++---------- .../boards/components/modal/empty_state.js | 120 +++++---- .../javascripts/boards/components/modal/footer.js | 126 +++++----- .../javascripts/boards/components/modal/header.js | 130 +++++----- .../javascripts/boards/components/modal/index.js | 272 ++++++++++----------- .../javascripts/boards/components/modal/list.js | 268 ++++++++++---------- .../boards/components/modal/lists_dropdown.js | 102 ++++---- .../javascripts/boards/components/modal/tabs.js | 86 ++++--- .../boards/components/new_list_dropdown.js | 120 +++++---- .../boards/components/sidebar/remove_issue.js | 98 ++++---- .../javascripts/boards/mixins/modal_mixins.js | 22 +- .../boards/mixins/sortable_default_options.js | 60 +++-- .../javascripts/boards/stores/boards_store.js | 216 ++++++++-------- .../javascripts/boards/stores/modal_store.js | 156 ++++++------ app/assets/javascripts/lib/utils/url_utility.js | 171 +++++++------ 18 files changed, 1232 insertions(+), 1269 deletions(-) diff --git a/app/assets/javascripts/boards/components/board.js b/app/assets/javascripts/boards/components/board.js index 93b8960da2e..239eeacf2d7 100644 --- a/app/assets/javascripts/boards/components/board.js +++ b/app/assets/javascripts/boards/components/board.js @@ -7,100 +7,98 @@ import boardBlankState from './board_blank_state'; require('./board_delete'); require('./board_list'); -(() => { - const Store = gl.issueBoards.BoardsStore; +const Store = gl.issueBoards.BoardsStore; - window.gl = window.gl || {}; - window.gl.issueBoards = window.gl.issueBoards || {}; +window.gl = window.gl || {}; +window.gl.issueBoards = window.gl.issueBoards || {}; - gl.issueBoards.Board = Vue.extend({ - template: '#js-board-template', - components: { - boardList, - 'board-delete': gl.issueBoards.BoardDelete, - boardBlankState, - }, - props: { - list: Object, - disabled: Boolean, - issueLinkBase: String, - rootPath: String, - }, - data () { - return { - detailIssue: Store.detail, - filter: Store.filter, - }; - }, - watch: { - filter: { - handler() { - this.list.page = 1; - this.list.getIssues(true); - }, - deep: true, +gl.issueBoards.Board = Vue.extend({ + template: '#js-board-template', + components: { + boardList, + 'board-delete': gl.issueBoards.BoardDelete, + boardBlankState, + }, + props: { + list: Object, + disabled: Boolean, + issueLinkBase: String, + rootPath: String, + }, + data () { + return { + detailIssue: Store.detail, + filter: Store.filter, + }; + }, + watch: { + filter: { + handler() { + this.list.page = 1; + this.list.getIssues(true); }, - detailIssue: { - handler () { - if (!Object.keys(this.detailIssue.issue).length) return; + deep: true, + }, + detailIssue: { + handler () { + if (!Object.keys(this.detailIssue.issue).length) return; - const issue = this.list.findIssue(this.detailIssue.issue.id); + const issue = this.list.findIssue(this.detailIssue.issue.id); - if (issue) { - const offsetLeft = this.$el.offsetLeft; - const boardsList = document.querySelectorAll('.boards-list')[0]; - const left = boardsList.scrollLeft - offsetLeft; - let right = (offsetLeft + this.$el.offsetWidth); + if (issue) { + const offsetLeft = this.$el.offsetLeft; + const boardsList = document.querySelectorAll('.boards-list')[0]; + const left = boardsList.scrollLeft - offsetLeft; + let right = (offsetLeft + this.$el.offsetWidth); - if (window.innerWidth > 768 && boardsList.classList.contains('is-compact')) { - // -290 here because width of boardsList is animating so therefore - // getting the width here is incorrect - // 290 is the width of the sidebar - right -= (boardsList.offsetWidth - 290); - } else { - right -= boardsList.offsetWidth; - } + if (window.innerWidth > 768 && boardsList.classList.contains('is-compact')) { + // -290 here because width of boardsList is animating so therefore + // getting the width here is incorrect + // 290 is the width of the sidebar + right -= (boardsList.offsetWidth - 290); + } else { + right -= boardsList.offsetWidth; + } - if (right - boardsList.scrollLeft > 0) { - $(boardsList).animate({ - scrollLeft: right - }, this.sortableOptions.animation); - } else if (left > 0) { - $(boardsList).animate({ - scrollLeft: offsetLeft - }, this.sortableOptions.animation); - } + if (right - boardsList.scrollLeft > 0) { + $(boardsList).animate({ + scrollLeft: right + }, this.sortableOptions.animation); + } else if (left > 0) { + $(boardsList).animate({ + scrollLeft: offsetLeft + }, this.sortableOptions.animation); } - }, - deep: true - } - }, - methods: { - showNewIssueForm() { - this.$refs['board-list'].showIssueForm = !this.$refs['board-list'].showIssueForm; - } - }, - mounted () { - this.sortableOptions = gl.issueBoards.getBoardSortableDefaultOptions({ - disabled: this.disabled, - group: 'boards', - draggable: '.is-draggable', - handle: '.js-board-handle', - onEnd: (e) => { - gl.issueBoards.onEnd(); + } + }, + deep: true + } + }, + methods: { + showNewIssueForm() { + this.$refs['board-list'].showIssueForm = !this.$refs['board-list'].showIssueForm; + } + }, + mounted () { + this.sortableOptions = gl.issueBoards.getBoardSortableDefaultOptions({ + disabled: this.disabled, + group: 'boards', + draggable: '.is-draggable', + handle: '.js-board-handle', + onEnd: (e) => { + gl.issueBoards.onEnd(); - if (e.newIndex !== undefined && e.oldIndex !== e.newIndex) { - const order = this.sortable.toArray(); - const list = Store.findList('id', parseInt(e.item.dataset.id, 10)); + if (e.newIndex !== undefined && e.oldIndex !== e.newIndex) { + const order = this.sortable.toArray(); + const list = Store.findList('id', parseInt(e.item.dataset.id, 10)); - this.$nextTick(() => { - Store.moveList(list, order); - }); - } + this.$nextTick(() => { + Store.moveList(list, order); + }); } - }); + } + }); - this.sortable = Sortable.create(this.$el.parentNode, this.sortableOptions); - }, - }); -})(); + this.sortable = Sortable.create(this.$el.parentNode, this.sortableOptions); + }, +}); diff --git a/app/assets/javascripts/boards/components/board_delete.js b/app/assets/javascripts/boards/components/board_delete.js index af621cfd57f..8a1b177bba8 100644 --- a/app/assets/javascripts/boards/components/board_delete.js +++ b/app/assets/javascripts/boards/components/board_delete.js @@ -2,22 +2,20 @@ import Vue from 'vue'; -(() => { - window.gl = window.gl || {}; - window.gl.issueBoards = window.gl.issueBoards || {}; +window.gl = window.gl || {}; +window.gl.issueBoards = window.gl.issueBoards || {}; - gl.issueBoards.BoardDelete = Vue.extend({ - props: { - list: Object - }, - methods: { - deleteBoard () { - $(this.$el).tooltip('hide'); +gl.issueBoards.BoardDelete = Vue.extend({ + props: { + list: Object + }, + methods: { + deleteBoard () { + $(this.$el).tooltip('hide'); - if (confirm('Are you sure you want to delete this list?')) { - this.list.destroy(); - } + if (confirm('Are you sure you want to delete this list?')) { + this.list.destroy(); } } - }); -})(); + } +}); diff --git a/app/assets/javascripts/boards/components/board_sidebar.js b/app/assets/javascripts/boards/components/board_sidebar.js index 3c080008244..004bac09f59 100644 --- a/app/assets/javascripts/boards/components/board_sidebar.js +++ b/app/assets/javascripts/boards/components/board_sidebar.js @@ -8,66 +8,64 @@ import Vue from 'vue'; require('./sidebar/remove_issue'); -(() => { - const Store = gl.issueBoards.BoardsStore; +const Store = gl.issueBoards.BoardsStore; - window.gl = window.gl || {}; - window.gl.issueBoards = window.gl.issueBoards || {}; +window.gl = window.gl || {}; +window.gl.issueBoards = window.gl.issueBoards || {}; - gl.issueBoards.BoardSidebar = Vue.extend({ - props: { - currentUser: Object - }, - data() { - return { - detail: Store.detail, - issue: {}, - list: {}, - }; - }, - computed: { - showSidebar () { - return Object.keys(this.issue).length; - } - }, - watch: { - detail: { - handler () { - if (this.issue.id !== this.detail.issue.id) { - $('.js-issue-board-sidebar', this.$el).each((i, el) => { - $(el).data('glDropdown').clearMenu(); - }); - } - - this.issue = this.detail.issue; - this.list = this.detail.list; - }, - deep: true - }, - issue () { - if (this.showSidebar) { - this.$nextTick(() => { - $('.right-sidebar').getNiceScroll(0).doScrollTop(0, 0); - $('.right-sidebar').getNiceScroll().resize(); +gl.issueBoards.BoardSidebar = Vue.extend({ + props: { + currentUser: Object + }, + data() { + return { + detail: Store.detail, + issue: {}, + list: {}, + }; + }, + computed: { + showSidebar () { + return Object.keys(this.issue).length; + } + }, + watch: { + detail: { + handler () { + if (this.issue.id !== this.detail.issue.id) { + $('.js-issue-board-sidebar', this.$el).each((i, el) => { + $(el).data('glDropdown').clearMenu(); }); } - } + + this.issue = this.detail.issue; + this.list = this.detail.list; + }, + deep: true }, - methods: { - closeSidebar () { - this.detail.issue = {}; + issue () { + if (this.showSidebar) { + this.$nextTick(() => { + $('.right-sidebar').getNiceScroll(0).doScrollTop(0, 0); + $('.right-sidebar').getNiceScroll().resize(); + }); } - }, - mounted () { - new IssuableContext(this.currentUser); - new MilestoneSelect(); - new gl.DueDateSelectors(); - new LabelsSelect(); - new Sidebar(); - gl.Subscription.bindAll('.subscription'); - }, - components: { - removeBtn: gl.issueBoards.RemoveIssueBtn, - }, - }); -})(); + } + }, + methods: { + closeSidebar () { + this.detail.issue = {}; + } + }, + mounted () { + new IssuableContext(this.currentUser); + new MilestoneSelect(); + new gl.DueDateSelectors(); + new LabelsSelect(); + new Sidebar(); + gl.Subscription.bindAll('.subscription'); + }, + components: { + removeBtn: gl.issueBoards.RemoveIssueBtn, + }, +}); diff --git a/app/assets/javascripts/boards/components/issue_card_inner.js b/app/assets/javascripts/boards/components/issue_card_inner.js index e48d3344a2b..fc154ee7b8b 100644 --- a/app/assets/javascripts/boards/components/issue_card_inner.js +++ b/app/assets/javascripts/boards/components/issue_card_inner.js @@ -1,141 +1,139 @@ import Vue from 'vue'; import eventHub from '../eventhub'; -(() => { - const Store = gl.issueBoards.BoardsStore; +const Store = gl.issueBoards.BoardsStore; - window.gl = window.gl || {}; - window.gl.issueBoards = window.gl.issueBoards || {}; +window.gl = window.gl || {}; +window.gl.issueBoards = window.gl.issueBoards || {}; - gl.issueBoards.IssueCardInner = Vue.extend({ - props: { - issue: { - type: Object, - required: true, - }, - issueLinkBase: { - type: String, - required: true, - }, - list: { - type: Object, - required: false, - default: () => ({}), - }, - rootPath: { - type: String, - required: true, - }, - updateFilters: { - type: Boolean, - required: false, - default: false, - }, +gl.issueBoards.IssueCardInner = Vue.extend({ + props: { + issue: { + type: Object, + required: true, }, - computed: { - cardUrl() { - return `${this.issueLinkBase}/${this.issue.id}`; - }, - assigneeUrl() { - return `${this.rootPath}${this.issue.assignee.username}`; - }, - assigneeUrlTitle() { - return `Assigned to ${this.issue.assignee.name}`; - }, - avatarUrlTitle() { - return `Avatar for ${this.issue.assignee.name}`; - }, - issueId() { - return `#${this.issue.id}`; - }, - showLabelFooter() { - return this.issue.labels.find(l => this.showLabel(l)) !== undefined; - }, + issueLinkBase: { + type: String, + required: true, }, - methods: { - showLabel(label) { - if (!this.list) return true; + list: { + type: Object, + required: false, + default: () => ({}), + }, + rootPath: { + type: String, + required: true, + }, + updateFilters: { + type: Boolean, + required: false, + default: false, + }, + }, + computed: { + cardUrl() { + return `${this.issueLinkBase}/${this.issue.id}`; + }, + assigneeUrl() { + return `${this.rootPath}${this.issue.assignee.username}`; + }, + assigneeUrlTitle() { + return `Assigned to ${this.issue.assignee.name}`; + }, + avatarUrlTitle() { + return `Avatar for ${this.issue.assignee.name}`; + }, + issueId() { + return `#${this.issue.id}`; + }, + showLabelFooter() { + return this.issue.labels.find(l => this.showLabel(l)) !== undefined; + }, + }, + methods: { + showLabel(label) { + if (!this.list) return true; - return !this.list.label || label.id !== this.list.label.id; - }, - filterByLabel(label, e) { - if (!this.updateFilters) return; + return !this.list.label || label.id !== this.list.label.id; + }, + filterByLabel(label, e) { + if (!this.updateFilters) return; - const filterPath = gl.issueBoards.BoardsStore.filter.path.split('&'); - const labelTitle = encodeURIComponent(label.title); - const param = `label_name[]=${labelTitle}`; - const labelIndex = filterPath.indexOf(param); - $(e.currentTarget).tooltip('hide'); + const filterPath = gl.issueBoards.BoardsStore.filter.path.split('&'); + const labelTitle = encodeURIComponent(label.title); + const param = `label_name[]=${labelTitle}`; + const labelIndex = filterPath.indexOf(param); + $(e.currentTarget).tooltip('hide'); - if (labelIndex === -1) { - filterPath.push(param); - } else { - filterPath.splice(labelIndex, 1); - } + if (labelIndex === -1) { + filterPath.push(param); + } else { + filterPath.splice(labelIndex, 1); + } - gl.issueBoards.BoardsStore.filter.path = filterPath.join('&'); + gl.issueBoards.BoardsStore.filter.path = filterPath.join('&'); - Store.updateFiltersUrl(); + Store.updateFiltersUrl(); - eventHub.$emit('updateTokens'); - }, - labelStyle(label) { - return { - backgroundColor: label.color, - color: label.textColor, - }; - }, + eventHub.$emit('updateTokens'); + }, + labelStyle(label) { + return { + backgroundColor: label.color, + color: label.textColor, + }; }, - template: ` -
-
-

-