diff options
50 files changed, 772 insertions, 480 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index bd29a266ccd..8ea1f082787 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -484,6 +484,9 @@ setup-test-env: build-qa-image: <<: *review-docker + variables: + <<: *review-docker-variables + GIT_DEPTH: "20" stage: prepare script: - time docker build --cache-from ${LATEST_QA_IMAGE} --tag ${QA_IMAGE} ./qa/ diff --git a/.prettierrc b/.prettierrc index 3384551aea5..5e2863a11f6 100644 --- a/.prettierrc +++ b/.prettierrc @@ -1,13 +1,5 @@ { "printWidth": 100, "singleQuote": true, - "trailingComma": "es5", - "overrides": [ - { - "files": ["**/app/**/*", "**/spec/**/*"], - "options": { - "trailingComma": "all" - } - } - ] + "trailingComma": "all" } diff --git a/Dangerfile b/Dangerfile index 715a2bcbbae..32f4b4d23c3 100644 --- a/Dangerfile +++ b/Dangerfile @@ -12,3 +12,4 @@ danger.import_dangerfile(path: 'danger/duplicate_yarn_dependencies') danger.import_dangerfile(path: 'danger/prettier') danger.import_dangerfile(path: 'danger/eslint') danger.import_dangerfile(path: 'danger/roulette') +danger.import_dangerfile(path: 'danger/single_codebase') diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 5ff8c4f5d2a..5db08bf2dc5 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.26.0 +1.27.0 diff --git a/app/assets/javascripts/diffs/components/compare_versions.vue b/app/assets/javascripts/diffs/components/compare_versions.vue index 0bf2dde8b96..fe49dfff10b 100644 --- a/app/assets/javascripts/diffs/components/compare_versions.vue +++ b/app/assets/javascripts/diffs/components/compare_versions.vue @@ -125,9 +125,9 @@ export default { > {{ __('Show latest version') }} </gl-button> - <a v-show="hasCollapsedFile" class="btn btn-default append-right-8" @click="expandAllFiles"> + <gl-button v-show="hasCollapsedFile" class="append-right-8" @click="expandAllFiles"> {{ __('Expand all') }} - </a> + </gl-button> <settings-dropdown /> </div> </div> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 1141a197c6a..0e779e1be9a 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -73,13 +73,23 @@ export default { if (!newVal && oldVal && !this.hasDiffLines) { this.handleLoadCollapsedDiff(); } + + this.setFileCollapsed({ filePath: this.file.file_path, collapsed: newVal }); + }, + 'file.viewer.collapsed': function setIsCollapsed(newVal) { + this.isCollapsed = newVal; }, }, created() { eventHub.$on(`loadCollapsedDiff/${this.file.file_hash}`, this.handleLoadCollapsedDiff); }, methods: { - ...mapActions('diffs', ['loadCollapsedDiff', 'assignDiscussionsToDiff', 'setRenderIt']), + ...mapActions('diffs', [ + 'loadCollapsedDiff', + 'assignDiscussionsToDiff', + 'setRenderIt', + 'setFileCollapsed', + ]), handleToggle() { if (!this.hasDiffLines) { this.handleLoadCollapsedDiff(); diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 57ddc923a3e..4a04216d893 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -344,5 +344,8 @@ export const toggleFullDiff = ({ dispatch, getters, state }, filePath) => { } }; +export const setFileCollapsed = ({ commit }, { filePath, collapsed }) => + commit(types.SET_FILE_COLLAPSED, { filePath, collapsed }); + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index b441b1de451..adf56eba3f8 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -27,3 +27,4 @@ export const TOGGLE_FILE_FINDER_VISIBLE = 'TOGGLE_FILE_FINDER_VISIBLE'; export const REQUEST_FULL_DIFF = 'REQUEST_FULL_DIFF'; export const RECEIVE_FULL_DIFF_SUCCESS = 'RECEIVE_FULL_DIFF_SUCCESS'; export const RECEIVE_FULL_DIFF_ERROR = 'RECEIVE_FULL_DIFF_ERROR'; +export const SET_FILE_COLLAPSED = 'SET_FILE_COLLAPSED'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 45187d93fef..856f4eaf00a 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -104,7 +104,10 @@ export default { [types.EXPAND_ALL_FILES](state) { state.diffFiles = state.diffFiles.map(file => ({ ...file, - collapsed: false, + viewer: { + ...file.viewer, + collapsed: false, + }, })); }, @@ -300,4 +303,11 @@ export default { }), }); }, + [types.SET_FILE_COLLAPSED](state, { filePath, collapsed }) { + const file = state.diffFiles.find(f => f.file_path === filePath); + + if (file && file.viewer) { + file.viewer.collapsed = collapsed; + } + }, }; diff --git a/app/assets/javascripts/emoji/index.js b/app/assets/javascripts/emoji/index.js index 36542315c4c..bb5085a1911 100644 --- a/app/assets/javascripts/emoji/index.js +++ b/app/assets/javascripts/emoji/index.js @@ -3,6 +3,7 @@ import createFlash from '~/flash'; import { s__ } from '~/locale'; import emojiAliases from 'emojis/aliases.json'; import axios from '../lib/utils/axios_utils'; +import csrf from '../lib/utils/csrf'; import AccessorUtilities from '../lib/utils/accessor'; @@ -24,7 +25,14 @@ export function initEmojiMap() { resolve(emojiMap); } else { // We load the JSON from server - axios + const axiosInstance = axios.create(); + + // If the static JSON file is on a CDN we don't want to send the default CSRF token + if (gon.asset_host) { + delete axiosInstance.defaults.headers.common[csrf.headerKey]; + } + + axiosInstance .get( `${gon.asset_host || ''}${gon.relative_url_root || ''}/-/emojis/${EMOJI_VERSION}/emojis.json`, diff --git a/app/assets/javascripts/environments/components/environment_item.vue b/app/assets/javascripts/environments/components/environment_item.vue index 1e89dce69cb..a092bdfbc6c 100644 --- a/app/assets/javascripts/environments/components/environment_item.vue +++ b/app/assets/javascripts/environments/components/environment_item.vue @@ -4,6 +4,7 @@ import _ from 'underscore'; import { GlTooltipDirective } from '@gitlab/ui'; import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import Icon from '~/vue_shared/components/icon.vue'; +import environmentItemMixin from 'ee_else_ce/environments/mixins/environment_item_mixin'; import ActionsComponent from './environment_actions.vue'; import ExternalUrlComponent from './environment_external_url.vue'; import StopComponent from './environment_stop.vue'; @@ -34,10 +35,10 @@ export default { TerminalButtonComponent, MonitoringButtonComponent, }, - directives: { GlTooltip: GlTooltipDirective, }, + mixins: [environmentItemMixin], props: { model: { @@ -467,9 +468,18 @@ export default { <div v-if="!model.isFolder" class="table-mobile-header" role="rowheader"> {{ s__('Environments|Environment') }} </div> + + <span v-if="shouldRenderDeployBoard" class="deploy-board-icon" @click="toggleDeployBoard"> + <icon :name="deployIconName" /> + </span> + <span v-if="!model.isFolder" class="environment-name table-mobile-content"> <a class="qa-environment-link" :href="environmentPath"> {{ model.name }} </a> + <span v-if="isProtected" class="badge badge-success"> + {{ s__('Environments|protected') }} + </span> </span> + <span v-else class="folder-name" role="button" @click="onClickFolder"> <icon :name="folderIconName" class="folder-icon" /> diff --git a/app/assets/javascripts/environments/mixins/environment_item_mixin.js b/app/assets/javascripts/environments/mixins/environment_item_mixin.js new file mode 100644 index 00000000000..2dfed36ec99 --- /dev/null +++ b/app/assets/javascripts/environments/mixins/environment_item_mixin.js @@ -0,0 +1,13 @@ +export default { + computed: { + deployIconName() { + return ''; + }, + shouldRenderDeployBoard() { + return false; + }, + }, + methods: { + toggleDeployBoard() {}, + }, +}; diff --git a/app/assets/stylesheets/pages/stat_graph.scss b/app/assets/stylesheets/pages/stat_graph.scss index d331edaa302..79186480605 100644 --- a/app/assets/stylesheets/pages/stat_graph.scss +++ b/app/assets/stylesheets/pages/stat_graph.scss @@ -25,6 +25,8 @@ } #contributors { + flex: 1; + .contributors-list { margin: 0 0 10px; list-style: none; diff --git a/app/views/groups/_home_panel.html.haml b/app/views/groups/_home_panel.html.haml index 39c0c113793..dbd01c3c61a 100644 --- a/app/views/groups/_home_panel.html.haml +++ b/app/views/groups/_home_panel.html.haml @@ -13,7 +13,7 @@ = visibility_level_icon(@group.visibility_level, fw: false, options: {class: 'icon'}) .home-panel-metadata.d-flex.align-items-center.text-secondary %span - = _("Group") + = _("Group ID: %{group_id}") % { group_id: @group.id } - if current_user %span.access-request-links.prepend-left-8 = render 'shared/members/access_request_links', source: @group diff --git a/changelogs/unreleased/10029-env-item.yml b/changelogs/unreleased/10029-env-item.yml new file mode 100644 index 00000000000..f4e742d3e17 --- /dev/null +++ b/changelogs/unreleased/10029-env-item.yml @@ -0,0 +1,5 @@ +--- +title: Removes EE differences for environment_item.vue +merge_request: +author: +type: other diff --git a/changelogs/unreleased/expose-group-id-on-home-panel.yml b/changelogs/unreleased/expose-group-id-on-home-panel.yml new file mode 100644 index 00000000000..1efe15a6e1a --- /dev/null +++ b/changelogs/unreleased/expose-group-id-on-home-panel.yml @@ -0,0 +1,5 @@ +--- +title: Expose group id on home panel +merge_request: 25897 +author: Peter Marko +type: added diff --git a/changelogs/unreleased/sh-rugged-commit-tree-entry.yml b/changelogs/unreleased/sh-rugged-commit-tree-entry.yml new file mode 100644 index 00000000000..bcefa2c7112 --- /dev/null +++ b/changelogs/unreleased/sh-rugged-commit-tree-entry.yml @@ -0,0 +1,5 @@ +--- +title: Bring back Rugged implementation of commit_tree_entry +merge_request: 25896 +author: +type: other diff --git a/config/karma.config.js b/config/karma.config.js index e1d7c30b1c2..23eae40dceb 100644 --- a/config/karma.config.js +++ b/config/karma.config.js @@ -26,7 +26,7 @@ webpackConfig.devtool = 'cheap-inline-source-map'; webpackConfig.plugins.push( new webpack.DefinePlugin({ 'process.env.BABEL_ENV': JSON.stringify(process.env.BABEL_ENV || process.env.NODE_ENV || null), - }) + }), ); const specFilters = argumentsParser @@ -37,7 +37,7 @@ const specFilters = argumentsParser memo.push(filter, filter.replace(/\/?$/, '/**/*.js')); return memo; }, - [] + [], ) .parse(process.argv).filterSpec; @@ -51,7 +51,7 @@ if (specFilters.length) { root: ROOT_PATH, matchBase: true, }) - .filter(path => path.endsWith('spec.js')) + .filter(path => path.endsWith('spec.js')), ); // flatten @@ -78,8 +78,8 @@ if (specFilters.length) { new webpack.ContextReplacementPlugin( /spec[\\\/]javascripts$/, path.join(ROOT_PATH, 'spec/javascripts'), - newContext - ) + newContext, + ), ); } @@ -112,13 +112,13 @@ module.exports = function(config) { preprocessors: { 'spec/javascripts/**/*.js': ['webpack', 'sourcemap'], }, - reporters: ['progress'], + reporters: ['mocha'], webpack: webpackConfig, webpackMiddleware: { stats: 'errors-only' }, }; if (process.env.CI) { - karmaConfig.reporters = ['mocha', 'junit']; + karmaConfig.reporters.push('junit'); karmaConfig.junitReporter = { outputFile: 'junit_karma.xml', useBrowserName: false, diff --git a/config/webpack.config.js b/config/webpack.config.js index 64e6ec49219..11970b620bc 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -251,7 +251,7 @@ module.exports = { } else { resource.request = path.join( ROOT_PATH, - 'app/assets/javascripts/vue_shared/components/empty_component.js' + 'app/assets/javascripts/vue_shared/components/empty_component.js', ); } }), @@ -267,7 +267,7 @@ module.exports = { const missingDeps = Array.from(compilation.missingDependencies); const nodeModulesPath = path.join(ROOT_PATH, 'node_modules'); const hasMissingNodeModules = missingDeps.some( - file => file.indexOf(nodeModulesPath) !== -1 + file => file.indexOf(nodeModulesPath) !== -1, ); // watch for changes to missing node_modules @@ -278,7 +278,7 @@ module.exports = { // report our auto-generated bundle count console.log( - `${autoEntriesCount} entries from '/pages' automatically added to webpack output.` + `${autoEntriesCount} entries from '/pages' automatically added to webpack output.`, ); callback(); diff --git a/danger/roulette/Dangerfile b/danger/roulette/Dangerfile index 6cf54d0f854..808bc96a0a0 100644 --- a/danger/roulette/Dangerfile +++ b/danger/roulette/Dangerfile @@ -58,7 +58,9 @@ changes = helper.changes_by_category changes.delete(:none) categories = changes.keys - [:unknown] -unless changes.empty? +# Single codebase MRs are reviewed using a slightly different process, so we +# disable the review roulette for such MRs. +if changes.any? && !gitlab.mr_labels.include?('single codebase') team = begin helper.project_team diff --git a/danger/single_codebase/Dangerfile b/danger/single_codebase/Dangerfile new file mode 100644 index 00000000000..a5938cd6783 --- /dev/null +++ b/danger/single_codebase/Dangerfile @@ -0,0 +1,56 @@ +def mention_single_codebase_approvers + frontend_maintainers = %w(@filipa @iamphill) + backend_maintainers = %w(@rspeicher @rymai @yorickpeterse @godfat) + + rows = [] + users = [] + + if gitlab.mr_labels.include?('frontend') + frontend_maintainer = frontend_maintainers.sample + + rows << "| ~frontend | `#{frontend_maintainer}`" + users << frontend_maintainer + end + + if gitlab.mr_labels.include?('backend') + backend_maintainer = backend_maintainers.sample + + rows << "| ~backend | `#{backend_maintainer}`" + users << backend_maintainer + end + + if rows.empty? + backup_maintainer = backend_maintainers.sample + + rows << "| ~frontend / ~backend | `#{backup_maintainer}`" + users << backup_maintainer + end + + markdown(<<~MARKDOWN.strip) + ## Single codebase changes + + This merge request contains changes related to the work of moving towards a + [single codebase](https://gitlab.com/groups/gitlab-org/-/epics/802) for + Community Edition and Enterprise Edition. These changes will need to be + reviewed and approved by the following engineers: + + | Category | Reviewer + |----------|--------- + #{rows.join("\n")} + + To make sure this happens, please follow these steps: + + 1. Add all of the mentioned users to the list of merge request approvals. + 2. Assign the merge request to the first person in the above list. + + If you are a reviewer, please follow these steps: + + 1. Review the merge request. If it is good to go, approve it. + 2. Once approved, assign to the next person in the above list. If you are + the last person in the list, merge the merge request. + MARKDOWN +end + +if gitlab.mr_labels.include?('single codebase') + mention_single_codebase_approvers +end diff --git a/doc/ci/triggers/README.md b/doc/ci/triggers/README.md index bceccf4d40d..398b017277f 100644 --- a/doc/ci/triggers/README.md +++ b/doc/ci/triggers/README.md @@ -17,6 +17,12 @@ The following methods of authentication are supported. A unique trigger token can be obtained when [adding a new trigger](#adding-a-new-trigger). +DANGER: **Danger:** +Passing plain text tokens in public projects is a security issue. Potential +attackers can impersonate the user that exposed their trigger token publicly in +their `.gitlab-ci.yml` file. Use [variables](../variables/README.md#variables) +to protect trigger tokens. + ## Adding a new trigger You can add a new trigger by going to your project's @@ -53,9 +59,6 @@ The action is irreversible. > > - Valid refs are only the branches and tags. If you pass a commit SHA as a ref, > it will not trigger a job. -> - If your project is public, passing the token in plain text is probably not the -> wisest idea, so you might want to use a -> [variable](../variables/README.md#variables) for that purpose. To trigger a job you need to send a `POST` request to GitLab's API endpoint: diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index f170323059a..a44f4b62a0e 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -367,10 +367,11 @@ job: - branches@gitlab-org/gitlab-ce except: - master@gitlab-org/gitlab-ce + - release/.*@gitlab-org/gitlab-ce ``` The above example will run `job` for all branches on `gitlab-org/gitlab-ce`, -except master. +except `master` and those with names prefixed with `release/`. If a job does not have an `only` rule, `only: ['branches', 'tags']` is set by default. If it doesn't have an `except` rule, it is empty. @@ -1756,7 +1757,7 @@ include: ``` All [nested includes](#nested-includes) will be executed in the scope of the target project, -so it is possible to used local (relative to target project), project, remote +so it is possible to use local (relative to target project), project, remote or template includes. #### `include:template` @@ -1792,9 +1793,17 @@ include: All nested includes will be executed without context as public user, so only another remote, or public project, or template is allowed. +NOTE: **Note:** +Changes to remote includes will not have effect on already created pipelines, +because the include is being evaluated at the time of pipeline creation. +This is when full definition of CI yaml is being expanded in order to create +pipeline with stages with jobs. You always retry job that is already created, +thus created after pipeline creation. To re-include all (thus re-evaluate the +configuration), you have to re-create pipeline. + #### Nested includes -> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/53903) in GitLab 11.7. +> [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/56836) in GitLab 11.9. Nested includes allow you to compose a set of includes. A total of 50 includes is allowed. diff --git a/doc/development/testing_guide/frontend_testing.md b/doc/development/testing_guide/frontend_testing.md index 5b66e513a76..9bfb1e69f9e 100644 --- a/doc/development/testing_guide/frontend_testing.md +++ b/doc/development/testing_guide/frontend_testing.md @@ -35,15 +35,16 @@ If your test exceeds that time, it will fail. If you cannot improve the performance of the tests, you can increase the timeout for a specific test using -[`jest.setTimeout`](https://jestjs.io/docs/en/jest-object#jestsettimeouttimeout). +[`setTestTimeout`](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/frontend/helpers/timeout.js). ```javascript -beforeAll(() => { - jest.setTimeout(500); -}); +import { setTestTimeout } from 'helpers/timeout'; describe('Component', () => { - // ... + it('does something amazing', () => { + setTestTimeout(500); + // ... + }); }); ``` @@ -281,25 +282,6 @@ Information on setting up and running RSpec integration tests with ## Gotchas -### Errors due to use of unsupported JavaScript features - -Similar errors will be thrown if you're using JavaScript features not yet -supported by the PhantomJS test runner which is used for both Karma and RSpec -tests. We polyfill some JavaScript objects for older browsers, but some -features are still unavailable: - -- Array.from -- Array.first -- Async functions -- Generators -- Array destructuring -- For..Of -- Symbol/Symbol.iterator -- Spread - -Until these are polyfilled appropriately, they should not be used. Please -update this list with additional unsupported features. - ### RSpec errors due to JavaScript By default RSpec unit tests will not run JavaScript in the headless browser diff --git a/doc/raketasks/backup_restore.md b/doc/raketasks/backup_restore.md index dcd5e6e2245..c78cfc41678 100644 --- a/doc/raketasks/backup_restore.md +++ b/doc/raketasks/backup_restore.md @@ -100,6 +100,13 @@ the gitlab task runner pod via `kubectl`. Refer to [backing up a GitLab installa kubectl exec -it <gitlab task-runner pod> backup-utility ``` +Similarly to the Kubernetes case, if you have scaled out your GitLab +cluster to use multiple application servers, you should pick a +designated node (that won't be auto-scaled away) for running the +backup rake task. Because the backup rake task is tightly coupled to +the main Rails application, this is typically a node on which you're +also running Unicorn/Puma and/or Sidekiq. + Example output: ``` diff --git a/doc/user/instance_statistics/convdev.md b/doc/user/instance_statistics/convdev.md index 247be1fb392..2c9e0ecbf65 100644 --- a/doc/user/instance_statistics/convdev.md +++ b/doc/user/instance_statistics/convdev.md @@ -2,7 +2,7 @@ > [Introduced](https://gitlab.com/gitlab-org/gitlab-ce/issues/30469) in GitLab 9.3. -NOTE: **NOTE** +NOTE: **Note:** Your GitLab instance's [usage ping](../admin_area/settings/usage_statistics.md#usage-ping-core-only) must be activated in order to use this feature. The Conversational Development Index (ConvDev Index) gives you an overview of your entire diff --git a/lib/gitlab/danger/teammate.rb b/lib/gitlab/danger/teammate.rb index 4b822aa86c5..bfada512727 100644 --- a/lib/gitlab/danger/teammate.rb +++ b/lib/gitlab/danger/teammate.rb @@ -21,21 +21,21 @@ module Gitlab # Traintainers also count as reviewers def reviewer?(project, category) - capabilities(project) == "reviewer #{category}" || traintainer?(project, category) + capabilities(project).include?("reviewer #{category}") || traintainer?(project, category) end def traintainer?(project, category) - capabilities(project) == "trainee_maintainer #{category}" + capabilities(project).include?("trainee_maintainer #{category}") end def maintainer?(project, category) - capabilities(project) == "maintainer #{category}" + capabilities(project).include?("maintainer #{category}") end private def capabilities(project) - projects.fetch(project, '') + Array(projects.fetch(project, [])) end end end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 491e4b47196..e5bbd500e98 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -314,11 +314,16 @@ module Gitlab def tree_entry(path) return unless path.present? + commit_tree_entry(path) + end + + def commit_tree_entry(path) # We're only interested in metadata, so limit actual data to 1 byte # since Gitaly doesn't support "send no data" option. entry = @repository.gitaly_commit_client.tree_entry(id, path, 1) return unless entry + # To be compatible with the rugged format entry = entry.to_h entry.delete(:data) entry[:name] = File.basename(path) diff --git a/lib/gitlab/git/rugged_impl/commit.rb b/lib/gitlab/git/rugged_impl/commit.rb index 251802878c3..f6777dfa0c3 100644 --- a/lib/gitlab/git/rugged_impl/commit.rb +++ b/lib/gitlab/git/rugged_impl/commit.rb @@ -43,6 +43,30 @@ module Gitlab end end + override :commit_tree_entry + def commit_tree_entry(path) + if Feature.enabled?(:rugged_commit_tree_entry) + rugged_tree_entry(path) + else + super + end + end + + # Is this the same as Blob.find_entry_by_path ? + def rugged_tree_entry(path) + rugged_commit.tree.path(path) + rescue Rugged::TreeError + nil + end + + def rugged_commit + @rugged_commit ||= if raw_commit.is_a?(Rugged::Commit) + raw_commit + else + @repository.rev_parse_target(id) + end + end + def init_from_rugged(commit) author = commit.author committer = commit.committer diff --git a/lib/gitlab/git/rugged_impl/repository.rb b/lib/gitlab/git/rugged_impl/repository.rb index fe0120b1199..c0a91f59ab9 100644 --- a/lib/gitlab/git/rugged_impl/repository.rb +++ b/lib/gitlab/git/rugged_impl/repository.rb @@ -12,7 +12,7 @@ module Gitlab module Repository extend ::Gitlab::Utils::Override - FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor).freeze + FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry).freeze def alternate_object_directories relative_object_directories.map { |d| File.join(path, d) } diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index 48c113a8b14..0a371889af2 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -257,8 +257,7 @@ module Gitlab # This is this actual number of times this call was made. Used for information purposes only actual_call_count = increment_call_count("gitaly_#{call_site}_actual") - # Do no enforce limits in production - return if Rails.env.production? || ENV["GITALY_DISABLE_REQUEST_LIMITS"] + return unless enforce_gitaly_request_limits? # Check if this call is nested within a allow_n_plus_1_calls # block and skip check if it is @@ -275,6 +274,19 @@ module Gitlab raise TooManyInvocationsError.new(call_site, actual_call_count, max_call_count, max_stacks) end + def self.enforce_gitaly_request_limits? + # We typically don't want to enforce request limits in production + # However, we have some production-like test environments, i.e., ones + # where `Rails.env.production?` returns `true`. We do want to be able to + # check if the limit is being exceeded while testing in those environments + # In that case we can use a feature flag to indicate that we do want to + # enforce request limits. + return true if feature_enabled?('enforce_requests_limits') + + !(Rails.env.production? || ENV["GITALY_DISABLE_REQUEST_LIMITS"]) + end + private_class_method :enforce_gitaly_request_limits? + def self.allow_n_plus_1_calls return yield unless Gitlab::SafeRequestStore.active? diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 006ec1ec56f..4751b7264a5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3196,6 +3196,9 @@ msgstr "" msgid "Environments|You don't have any environments right now" msgstr "" +msgid "Environments|protected" +msgstr "" + msgid "Epic" msgstr "" @@ -3789,6 +3792,9 @@ msgstr "" msgid "Group ID" msgstr "" +msgid "Group ID: %{group_id}" +msgstr "" + msgid "Group Runners" msgstr "" diff --git a/package.json b/package.json index 7cdb11dca14..632a1b90289 100644 --- a/package.json +++ b/package.json @@ -169,7 +169,7 @@ "nodemon": "^1.18.9", "pixelmatch": "^4.0.2", "postcss": "^7.0.14", - "prettier": "1.16.1", + "prettier": "1.16.4", "stylelint": "^9.10.1", "stylelint-config-recommended": "^2.1.0", "stylelint-scss": "^3.5.3", @@ -342,6 +342,10 @@ module QA module Specs autoload :Config, 'qa/specs/config' autoload :Runner, 'qa/specs/runner' + + module Helpers + autoload :Quarantine, 'qa/specs/helpers/quarantine' + end end ## diff --git a/qa/qa/specs/helpers/quarantine.rb b/qa/qa/specs/helpers/quarantine.rb new file mode 100644 index 00000000000..52cb05fcd13 --- /dev/null +++ b/qa/qa/specs/helpers/quarantine.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'rspec/core' + +module QA::Specs::Helpers + module Quarantine + include RSpec::Core::Pending + + extend self + + def configure_rspec + RSpec.configure do |config| + config.before(:context, :quarantine) do + Quarantine.skip_or_run_quarantined_contexts(config.inclusion_filter.rules, self.class) + end + + config.before do |example| + Quarantine.skip_or_run_quarantined_tests_or_contexts(config.inclusion_filter.rules, example) + end + end + end + + # Skip tests in quarantine unless we explicitly focus on them. + def skip_or_run_quarantined_tests_or_contexts(filters, example) + if filters.key?(:quarantine) + included_filters = filters_other_than_quarantine(filters) + + # If :quarantine is focused, skip the test/context unless its metadata + # includes quarantine and any other filters + # E.g., Suppose a test is tagged :smoke and :quarantine, and another is tagged + # :ldap and :quarantine. If we wanted to run just quarantined smoke tests + # using `--tag quarantine --tag smoke`, without this check we'd end up + # running that ldap test as well because of the :quarantine metadata. + # We could use an exclusion filter, but this way the test report will list + # the quarantined tests when they're not run so that we're aware of them + skip("Only running tests tagged with :quarantine and any of #{included_filters.keys}") if should_skip_when_focused?(example.metadata, included_filters) + else + skip('In quarantine') if example.metadata.key?(:quarantine) + end + end + + # Skip the entire context if a context is quarantined. This avoids running + # before blocks unnecessarily. + def skip_or_run_quarantined_contexts(filters, example) + return unless example.metadata.key?(:quarantine) + + skip_or_run_quarantined_tests_or_contexts(filters, example) + end + + def filters_other_than_quarantine(filter) + filter.reject { |key, _| key == :quarantine } + end + + # Checks if a test or context should be skipped. + # + # Returns true if + # - the metadata does not includes the :quarantine tag + # or if + # - the metadata includes the :quarantine tag + # - and the filter includes other tags that aren't in the metadata + def should_skip_when_focused?(metadata, included_filters) + return true unless metadata.key?(:quarantine) + return false if included_filters.empty? + + (metadata.keys & included_filters.keys).empty? + end + end +end diff --git a/qa/spec/spec_helper.rb b/qa/spec/spec_helper.rb index cbdd6e881b1..be13c3fb683 100644 --- a/qa/spec/spec_helper.rb +++ b/qa/spec/spec_helper.rb @@ -6,16 +6,10 @@ require 'rspec/retry' end RSpec.configure do |config| - config.before(:context) do - if self.class.metadata.keys.include?(:quarantine) - skip_or_run_quarantined_tests(self.class.metadata.keys, config.inclusion_filter.rules.keys) - end - end + QA::Specs::Helpers::Quarantine.configure_rspec config.before do |example| QA::Runtime::Logger.debug("Starting test: #{example.full_description}") if QA::Runtime::Env.debug? - - skip_or_run_quarantined_tests(example.metadata.keys, config.inclusion_filter.rules.keys) end config.expect_with :rspec do |expectations| @@ -44,42 +38,3 @@ RSpec.configure do |config| example.run_with_retry retry: retry_times end end - -# Skip tests in quarantine unless we explicitly focus on them. -# Skip the entire context if a context is tagged. This avoids running before -# blocks unnecessarily. -# If quarantine is focussed, skip tests/contexts that have other metadata -# unless they're also focussed. This lets us run quarantined tests in a -# particular category without running tests in other categories. -# E.g., if a test is tagged 'smoke' and 'quarantine', and another is tagged -# 'ldap' and 'quarantine', if we wanted to run just quarantined smoke tests -# using `--tag quarantine --tag smoke`, without this check we'd end up -# running that ldap test as well. -# We could use an exclusion filter, but this way the test report will list -# the quarantined tests when they're not run so that we're aware of them -def skip_or_run_quarantined_tests(metadata_keys, filter_keys) - included_filters = filters_other_than_quarantine(filter_keys) - - if filter_keys.include?(:quarantine) - skip("Only running tests tagged with :quarantine and any of #{included_filters}") unless quarantine_and_optional_other_tag?(metadata_keys, included_filters) - else - skip('In quarantine') if metadata_keys.include?(:quarantine) - end -end - -def filters_other_than_quarantine(filter_keys) - filter_keys.reject { |key| key == :quarantine } -end - -# Checks if a test has the 'quarantine' tag and other tags in the inclusion filter. -# -# Returns true if -# - the metadata includes the quarantine tag -# - and the metadata and inclusion filter both have any other tag -# - or no other tags are in the inclusion filter -def quarantine_and_optional_other_tag?(metadata_keys, included_filters) - return false unless metadata_keys.include? :quarantine - return true if included_filters.empty? - - included_filters.any? { |key| metadata_keys.include? key } -end diff --git a/qa/spec/spec_helper_spec.rb b/qa/spec/spec_helper_spec.rb deleted file mode 100644 index 27ec1ec80fe..00000000000 --- a/qa/spec/spec_helper_spec.rb +++ /dev/null @@ -1,355 +0,0 @@ -# frozen_string_literal: true - -describe 'rspec config tests' do - let(:group) do - RSpec.describe do - shared_examples 'passing tests' do - example 'not in quarantine' do - end - example 'in quarantine', :quarantine do - end - end - - context 'default' do - it_behaves_like 'passing tests' - end - - context 'foo', :foo do - it_behaves_like 'passing tests' - end - - context 'quarantine', :quarantine do - it_behaves_like 'passing tests' - end - - context 'bar quarantine', :bar, :quarantine do - it_behaves_like 'passing tests' - end - end - end - - let(:group_2) do - RSpec.describe do - before(:all) do - @expectations = [1, 2, 3] - end - - example 'not in quarantine' do - expect(@expectations.shift).to be(3) - end - - example 'in quarantine', :quarantine do - expect(@expectations.shift).to be(3) - end - end - end - - context 'with no tags focussed' do - before do - group.run - end - - context 'in a context tagged :foo' do - it 'skips tests in quarantine' do - context = group.children.find { |c| c.description == "foo" } - examples = context.descendant_filtered_examples - expect(examples.count).to eq(2) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - expect(ex.execution_result.pending_message).to eq('In quarantine') - end - end - - context 'in an untagged context' do - it 'skips tests in quarantine' do - context = group.children.find { |c| c.description == "default" } - examples = context.descendant_filtered_examples - expect(examples.count).to eq(2) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - expect(ex.execution_result.pending_message).to eq('In quarantine') - end - end - - context 'in a context tagged :quarantine' do - it 'skips all tests' do - context = group.children.find { |c| c.description == "quarantine" } - examples = context.descendant_filtered_examples - expect(examples.count).to eq(2) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - expect(ex.execution_result.pending_message).to eq('In quarantine') - end - end - end - - context 'with :quarantine focussed' do - before do - RSpec.configure do |config| - config.inclusion_filter = :quarantine - end - - group.run - end - after do - RSpec.configure do |config| - config.inclusion_filter.clear - end - end - - context 'in an untagged context' do - it 'only runs quarantined tests' do - context = group.children.find { |c| c.description == "default" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(1) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - end - end - - context 'in a context tagged :foo' do - it 'only runs quarantined tests' do - context = group.children.find { |c| c.description == "foo" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(1) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - end - end - - context 'in a context tagged :quarantine' do - it 'runs all tests' do - context = group.children.find { |c| c.description == "quarantine" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(2) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - end - end - end - - context 'with a non-quarantine tag (:foo) focussed' do - before do - RSpec.configure do |config| - config.inclusion_filter = :foo - end - - group.run - end - after do - RSpec.configure do |config| - config.inclusion_filter.clear - end - end - - context 'in an untagged context' do - it 'runs no tests' do - context = group.children.find { |c| c.description == "default" } - expect(context.descendant_filtered_examples.count).to eq(0) - end - end - - context 'in a context tagged :foo' do - it 'skips quarantined tests' do - context = group.children.find { |c| c.description == "foo" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(2) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - expect(ex.execution_result.pending_message).to eq('In quarantine') - end - end - - context 'in a context tagged :quarantine' do - it 'runs no tests' do - context = group.children.find { |c| c.description == "quarantine" } - expect(context.descendant_filtered_examples.count).to eq(0) - end - end - end - - context 'with :quarantine and a non-quarantine tag (:foo) focussed' do - before do - RSpec.configure do |config| - config.inclusion_filter = { quarantine: true, foo: true } - end - - group.run - end - after do - RSpec.configure do |config| - config.inclusion_filter.clear - end - end - - context 'in an untagged context' do - it 'ignores untagged tests and skips tests even if in quarantine' do - context = group.children.find { |c| c.description == "default" } - examples = context.descendant_filtered_examples - expect(examples.count).to eq(1) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - end - end - - context 'in a context tagged :foo' do - it 'only runs quarantined tests' do - context = group.children.find { |c| c.description == "foo" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(2) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - end - end - - context 'in a context tagged :quarantine' do - it 'skips all tests' do - context = group.children.find { |c| c.description == "quarantine" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(2) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - end - end - - context 'in a context tagged :bar and :quarantine' do - it 'skips all tests' do - context = group.children.find { |c| c.description == "quarantine" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(2) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - end - end - end - - context 'with :quarantine and multiple non-quarantine tags focussed' do - before do - RSpec.configure do |config| - config.inclusion_filter = { bar: true, foo: true, quarantine: true } - end - - group.run - end - after do - RSpec.configure do |config| - config.inclusion_filter.clear - end - end - - context 'in a context tagged :foo' do - it 'only runs quarantined tests' do - context = group.children.find { |c| c.description == "foo" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(2) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - expect(ex.execution_result.pending_message).to eq('Only running tests tagged with :quarantine and any of [:bar, :foo]') - end - end - - context 'in a context tagged :quarantine' do - it 'skips all tests' do - context = group.children.find { |c| c.description == "quarantine" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(2) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - expect(ex.execution_result.pending_message).to eq('Only running tests tagged with :quarantine and any of [:bar, :foo]') - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:pending) - expect(ex.execution_result.pending_message).to eq('Only running tests tagged with :quarantine and any of [:bar, :foo]') - end - end - - context 'in a context tagged :bar and :quarantine' do - it 'runs all tests' do - context = group.children.find { |c| c.description == "bar quarantine" } - examples = context.descendant_filtered_examples - expect(examples.count).to be(2) - - ex = examples.find { |e| e.description == "in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - - ex = examples.find { |e| e.description == "not in quarantine" } - expect(ex.execution_result.status).to eq(:passed) - end - end - end - - context 'rspec retry' do - context 'in an untagged context' do - before do - group_2.run - end - - it 'should run example :retry times' do - examples = group_2.descendant_filtered_examples - ex = examples.find { |e| e.description == 'not in quarantine' } - expect(ex.execution_result.status).to eq(:passed) - end - end - - context 'with :quarantine focussed' do - before do - RSpec.configure do |config| - config.inclusion_filter = :quarantine - end - group_2.run - end - - after do - RSpec.configure do |config| - config.inclusion_filter.clear - end - end - - it 'should run example once only' do - examples = group_2.descendant_filtered_examples - ex = examples.find { |e| e.description == 'in quarantine' } - expect(ex.execution_result.status).to eq(:failed) - end - end - end -end diff --git a/qa/spec/specs/helpers/quarantine_spec.rb b/qa/spec/specs/helpers/quarantine_spec.rb new file mode 100644 index 00000000000..78beda39b5e --- /dev/null +++ b/qa/spec/specs/helpers/quarantine_spec.rb @@ -0,0 +1,271 @@ +# frozen_string_literal: true + +require 'rspec/core/sandbox' + +# We need a reporter for internal tests that's different from the reporter for +# external tests otherwise the results will be mixed up. We don't care about +# most reporting, but we do want to know if a test fails +class RaiseOnFailuresReporter < RSpec::Core::NullReporter + def self.example_failed(example) + raise example.exception + end +end + +# We use an example group wrapper to prevent the state of internal tests +# expanding into the global state +# See: https://github.com/rspec/rspec-core/issues/2603 +def describe_successfully(*args, &describe_body) + example_group = RSpec.describe(*args, &describe_body) + ran_successfully = example_group.run RaiseOnFailuresReporter + expect(ran_successfully).to eq true + example_group +end + +RSpec.configure do |c| + c.around do |ex| + RSpec::Core::Sandbox.sandboxed do |config| + # If there is an example-within-an-example, we want to make sure the inner example + # does not get a reference to the outer example (the real spec) if it calls + # something like `pending` + config.before(:context) { RSpec.current_example = nil } + + config.color_mode = :off + + # Load airborne again to avoid "undefined method `match_expected_default?'" errors + # that happen because a hook calls a method added via a custom RSpec setting + # that is removed when the RSpec configuration is sandboxed. + # If this needs to be changed (e.g., to load other libraries as well), see + # this discussion for alternative solutions: + # https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25223#note_143392053 + load 'airborne.rb' + + ex.run + end + end +end + +describe QA::Specs::Helpers::Quarantine do + describe '.skip_or_run_quarantined_contexts' do + context 'with no tag focused' do + before do + described_class.configure_rspec + end + + it 'skips before hooks of quarantined contexts' do + executed_hooks = [] + + group = describe_successfully('quarantine', :quarantine) do + before(:all) do + executed_hooks << :before_all + end + before do + executed_hooks << :before + end + example {} + end + + expect(executed_hooks).to eq [] + expect(group.descendant_filtered_examples.first.execution_result.status).to eq(:pending) + expect(group.descendant_filtered_examples.first.execution_result.pending_message) + .to eq('In quarantine') + end + + it 'executes before hooks of non-quarantined contexts' do + executed_hooks = [] + + group = describe_successfully do + before(:all) do + executed_hooks << :before_all + end + before do + executed_hooks << :before + end + example {} + end + + expect(executed_hooks).to eq [:before_all, :before] + expect(group.descendant_filtered_examples.first.execution_result.status).to eq(:passed) + end + end + + context 'with :quarantine focused' do + before do + described_class.configure_rspec + RSpec.configure do |c| + c.filter_run :quarantine + end + end + + it 'executes before hooks of quarantined contexts' do + executed_hooks = [] + + group = describe_successfully('quarantine', :quarantine) do + before(:all) do + executed_hooks << :before_all + end + before do + executed_hooks << :before + end + example {} + end + + expect(executed_hooks).to eq [:before_all, :before] + expect(group.descendant_filtered_examples.first.execution_result.status).to eq(:passed) + end + + it 'skips before hooks of non-quarantined contexts' do + executed_hooks = [] + + group = describe_successfully do + before(:all) do + executed_hooks << :before_all + end + before do + executed_hooks << :before + end + example {} + end + + expect(executed_hooks).to eq [] + expect(group.descendant_filtered_examples.first).to be_nil + end + end + end + + describe '.skip_or_run_quarantined_tests' do + context 'with no tag focused' do + before do + described_class.configure_rspec + end + + it 'skips quarantined tests' do + group = describe_successfully do + it('is pending', :quarantine) {} + end + + expect(group.examples.first.execution_result.status).to eq(:pending) + expect(group.examples.first.execution_result.pending_message) + .to eq('In quarantine') + end + + it 'executes non-quarantined tests' do + group = describe_successfully do + example {} + end + + expect(group.examples.first.execution_result.status).to eq(:passed) + end + end + + context 'with :quarantine focused' do + before do + described_class.configure_rspec + RSpec.configure do |c| + c.filter_run :quarantine + end + end + + it 'executes quarantined tests' do + group = describe_successfully do + it('passes', :quarantine) {} + end + + expect(group.examples.first.execution_result.status).to eq(:passed) + end + + it 'ignores non-quarantined tests' do + group = describe_successfully do + example {} + end + + expect(group.examples.first.execution_result.status).to be_nil + end + end + + context 'with a non-quarantine tag focused' do + before do + described_class.configure_rspec + RSpec.configure do |c| + c.filter_run :foo + end + end + + it 'ignores non-quarantined non-focused tests' do + group = describe_successfully do + example {} + end + + expect(group.examples.first.execution_result.status).to be_nil + end + + it 'executes non-quarantined focused tests' do + group = describe_successfully do + it('passes', :foo) {} + end + + expect(group.examples.first.execution_result.status).to be(:passed) + end + + it 'ignores quarantined tests' do + group = describe_successfully do + it('is ignored', :quarantine) {} + end + + expect(group.examples.first.execution_result.status).to be_nil + end + + it 'skips quarantined focused tests' do + group = describe_successfully do + it('is pending', :quarantine, :foo) {} + end + + expect(group.examples.first.execution_result.status).to be(:pending) + expect(group.examples.first.execution_result.pending_message) + .to eq('In quarantine') + end + end + + context 'with :quarantine and non-quarantine tags focused' do + before do + described_class.configure_rspec + RSpec.configure do |c| + c.filter_run :foo, :bar, :quarantine + end + end + + it 'ignores non-quarantined non-focused tests' do + group = describe_successfully do + example {} + end + + expect(group.examples.first.execution_result.status).to be_nil + end + + it 'skips non-quarantined focused tests' do + group = describe_successfully do + it('is pending', :foo) {} + end + + expect(group.examples.first.execution_result.status).to be(:pending) + expect(group.examples.first.execution_result.pending_message) + .to eq('Only running tests tagged with :quarantine and any of [:bar, :foo]') + end + + it 'skips quarantined non-focused tests' do + group = describe_successfully do + it('is pending', :quarantine) {} + end + + expect(group.examples.first.execution_result.status).to be(:pending) + end + + it 'executes quarantined focused tests' do + group = describe_successfully do + it('passes', :quarantine, :foo) {} + end + + expect(group.examples.first.execution_result.status).to be(:passed) + end + end + end +end diff --git a/scripts/frontend/postinstall.js b/scripts/frontend/postinstall.js index 682039a41b3..94977e459e3 100644 --- a/scripts/frontend/postinstall.js +++ b/scripts/frontend/postinstall.js @@ -13,7 +13,7 @@ if (process.platform === 'darwin') { ensure that it is supported by the fsevents library. You can try installing again with \`${chalk.cyan('yarn install --force')}\` - `) + `), ); process.exit(1); } diff --git a/scripts/frontend/prettier.js b/scripts/frontend/prettier.js index ffb09ea9779..bf0e98da139 100644 --- a/scripts/frontend/prettier.js +++ b/scripts/frontend/prettier.js @@ -32,7 +32,7 @@ let globDir = process.argv[3] || ''; if (globDir && globDir.charAt(globDir.length - 1) !== '/') globDir += '/'; console.log( - `Loading all ${allFiles ? '' : 'staged '}files ${globDir ? `within ${globDir} ` : ''}...` + `Loading all ${allFiles ? '' : 'staged '}files ${globDir ? `within ${globDir} ` : ''}...`, ); const globPatterns = matchExtensions.map(ext => `${globDir}**/*.${ext}`); @@ -105,7 +105,7 @@ Promise.all(matchedFiles.map(checkFileWithPrettierConfig)) .then(() => { const failAction = shouldSave ? 'fixed' : 'failed'; console.log( - `\nSummary:\n ${matchedCount} files processed (${passedCount} passed, ${failedCount} ${failAction}, ${ignoredCount} ignored)\n` + `\nSummary:\n ${matchedCount} files processed (${passedCount} passed, ${failedCount} ${failAction}, ${ignoredCount} ignored)\n`, ); if (didWarn) process.exit(1); diff --git a/spec/features/merge_request/user_views_diffs_spec.rb b/spec/features/merge_request/user_views_diffs_spec.rb index 0434db04113..74342b16cb2 100644 --- a/spec/features/merge_request/user_views_diffs_spec.rb +++ b/spec/features/merge_request/user_views_diffs_spec.rb @@ -34,6 +34,16 @@ describe 'User views diffs', :js do expect(page).not_to have_selector('.mr-loading-status .loading', visible: true) end + it 'expands all diffs' do + first('#a5cc2925ca8258af241be7e5b0381edf30266302 .js-file-title').click + + expect(page).to have_button('Expand all') + + click_button 'Expand all' + + expect(page).not_to have_button('Expand all') + end + context 'when in the inline view' do include_examples 'unfold diffs' end diff --git a/spec/javascripts/diffs/components/diff_file_spec.js b/spec/javascripts/diffs/components/diff_file_spec.js index ba04c8c4a4c..d9b298e84da 100644 --- a/spec/javascripts/diffs/components/diff_file_spec.js +++ b/spec/javascripts/diffs/components/diff_file_spec.js @@ -109,6 +109,31 @@ describe('DiffFile', () => { done(); }); }); + + it('should update store state', done => { + spyOn(vm.$store, 'dispatch'); + + vm.isCollapsed = true; + + vm.$nextTick(() => { + expect(vm.$store.dispatch).toHaveBeenCalledWith('diffs/setFileCollapsed', { + filePath: vm.file.file_path, + collapsed: true, + }); + + done(); + }); + }); + + it('updates local state when changing file state', done => { + vm.file.viewer.collapsed = true; + + vm.$nextTick(() => { + expect(vm.isCollapsed).toBe(true); + + done(); + }); + }); }); }); diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index 070bfb2ccd0..6c637097893 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -35,6 +35,7 @@ import actions, { receiveFullDiffError, fetchFullDiff, toggleFullDiff, + setFileCollapsed, } from '~/diffs/store/actions'; import eventHub from '~/notes/event_hub'; import * as types from '~/diffs/store/mutation_types'; @@ -977,4 +978,17 @@ describe('DiffsStoreActions', () => { ); }); }); + + describe('setFileCollapsed', () => { + it('commits SET_FILE_COLLAPSED', done => { + testAction( + setFileCollapsed, + { filePath: 'test', collapsed: true }, + null, + [{ type: types.SET_FILE_COLLAPSED, payload: { filePath: 'test', collapsed: true } }], + [], + done, + ); + }); + }); }); diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 270e7d75666..5556a994a14 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -58,13 +58,15 @@ describe('DiffsStoreMutations', () => { describe('EXPAND_ALL_FILES', () => { it('should change the collapsed prop from diffFiles', () => { const diffFile = { - collapsed: true, + viewer: { + collapsed: true, + }, }; const state = { expandAllFiles: true, diffFiles: [diffFile] }; mutations[types.EXPAND_ALL_FILES](state); - expect(state.diffFiles[0].collapsed).toEqual(false); + expect(state.diffFiles[0].viewer.collapsed).toEqual(false); }); }); @@ -742,4 +744,16 @@ describe('DiffsStoreMutations', () => { expect(state.diffFiles[0].isShowingFullFile).toBe(true); }); }); + + describe('SET_FILE_COLLAPSED', () => { + it('sets collapsed', () => { + const state = { + diffFiles: [{ file_path: 'test', viewer: { collapsed: false } }], + }; + + mutations[types.SET_FILE_COLLAPSED](state, { filePath: 'test', collapsed: true }); + + expect(state.diffFiles[0].viewer.collapsed).toBe(true); + }); + }); }); diff --git a/spec/lib/gitlab/danger/teammate_spec.rb b/spec/lib/gitlab/danger/teammate_spec.rb new file mode 100644 index 00000000000..4bc0a4c1398 --- /dev/null +++ b/spec/lib/gitlab/danger/teammate_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +describe Gitlab::Danger::Teammate do + subject { described_class.new({ 'projects' => projects }) } + let(:projects) { { project => capabilities } } + let(:project) { double } + + describe 'multiple roles project project' do + let(:capabilities) { ['reviewer backend', 'maintainer frontend', 'trainee_maintainer database'] } + + it '#reviewer? supports multiple roles per project' do + expect(subject.reviewer?(project, 'backend')).to be_truthy + end + + it '#traintainer? supports multiple roles per project' do + expect(subject.traintainer?(project, 'database')).to be_truthy + end + + it '#maintainer? supports multiple roles per project' do + expect(subject.maintainer?(project, 'frontend')).to be_truthy + end + end + + describe 'one role project project' do + let(:capabilities) { 'reviewer backend' } + + it '#reviewer? supports one role per project' do + expect(subject.reviewer?(project, 'backend')).to be_truthy + end + + it '#traintainer? supports one role per project' do + expect(subject.traintainer?(project, 'database')).to be_falsey + end + + it '#maintainer? supports one role per project' do + expect(subject.maintainer?(project, 'frontend')).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index cf12baf1a93..f1acb1d9bc4 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -149,11 +149,21 @@ describe Gitlab::GitalyClient do end end - context 'when RequestStore is enabled', :request_store do + context 'when RequestStore is enabled and the maximum number of calls is not enforced by a feature flag', :request_store do + before do + stub_feature_flags(gitaly_enforce_requests_limits: false) + end + it 'allows up the maximum number of allowed calls' do expect { call_gitaly(Gitlab::GitalyClient::MAXIMUM_GITALY_CALLS) }.not_to raise_error end + it 'allows the maximum number of calls to be exceeded if GITALY_DISABLE_REQUEST_LIMITS is set' do + stub_env('GITALY_DISABLE_REQUEST_LIMITS', 'true') + + expect { call_gitaly(Gitlab::GitalyClient::MAXIMUM_GITALY_CALLS + 1) }.not_to raise_error + end + context 'when the maximum number of calls has been reached' do before do call_gitaly(Gitlab::GitalyClient::MAXIMUM_GITALY_CALLS) @@ -189,6 +199,32 @@ describe Gitlab::GitalyClient do end end + context 'in production and when RequestStore is enabled', :request_store do + before do + allow(Rails.env).to receive(:production?).and_return(true) + end + + context 'when the maximum number of calls is enforced by a feature flag' do + before do + stub_feature_flags(gitaly_enforce_requests_limits: true) + end + + it 'does not allow the maximum number of calls to be exceeded' do + expect { call_gitaly(Gitlab::GitalyClient::MAXIMUM_GITALY_CALLS + 1) }.to raise_error(Gitlab::GitalyClient::TooManyInvocationsError) + end + end + + context 'when the maximum number of calls is not enforced by a feature flag' do + before do + stub_feature_flags(gitaly_enforce_requests_limits: false) + end + + it 'allows the maximum number of calls to be exceeded' do + expect { call_gitaly(Gitlab::GitalyClient::MAXIMUM_GITALY_CALLS + 1) }.not_to raise_error + end + end + end + context 'when RequestStore is not active' do it 'does not raise errors when the maximum number of allowed calls is exceeded' do expect { call_gitaly(Gitlab::GitalyClient::MAXIMUM_GITALY_CALLS + 2) }.not_to raise_error diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index baad8352185..9d4e18534ae 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -542,7 +542,7 @@ eos end end - describe '#uri_type' do + shared_examples '#uri_type' do it 'returns the URI type at the given path' do expect(commit.uri_type('files/html')).to be(:tree) expect(commit.uri_type('files/images/logo-black.png')).to be(:raw) @@ -561,6 +561,20 @@ eos end end + describe '#uri_type with Gitaly enabled' do + it_behaves_like "#uri_type" + end + + describe '#uri_type with Rugged enabled', :enable_rugged do + it 'calls out to the Rugged implementation' do + allow_any_instance_of(Rugged::Tree).to receive(:path).with('files/html').and_call_original + + commit.uri_type('files/html') + end + + it_behaves_like '#uri_type' + end + describe '.from_hash' do let(:new_commit) { described_class.from_hash(commit.to_hash, project) } diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index cd85151ec1b..52ce4d66b4a 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -237,6 +237,14 @@ describe API::Internal do expect(json_response['name']).to eq(user.name) end + + it 'responds successfully when a user is not found' do + get(api("/internal/discover"), params: { username: 'noone', secret_token: secret_token }) + + expect(response).to have_gitlab_http_status(200) + + expect(response.body).to eq('null') + end end describe "GET /internal/authorized_keys" do diff --git a/spec/views/groups/_home_panel.html.haml_spec.rb b/spec/views/groups/_home_panel.html.haml_spec.rb new file mode 100644 index 00000000000..91c5ca261b9 --- /dev/null +++ b/spec/views/groups/_home_panel.html.haml_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' + +describe 'groups/_home_panel' do + let(:group) { create(:group) } + + before do + assign(:group, group) + end + + it 'renders the group ID' do + render + + expect(rendered).to have_content("Group ID: #{group.id}") + end +end diff --git a/yarn.lock b/yarn.lock index 1ac6b322469..3cb0fea64fc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8086,16 +8086,16 @@ prepend-http@^2.0.0: resolved "https://registry.yarnpkg.com/prepend-http/-/prepend-http-2.0.0.tgz#e92434bfa5ea8c19f41cdfd401d741a3c819d897" integrity sha1-6SQ0v6XqjBn0HN/UAddBo8gZ2Jc= -prettier@1.16.1: - version "1.16.1" - resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.16.1.tgz#534c2c9d7853f8845e5e078384e71973bd74089f" - integrity sha512-XXUITwIkGb3CPJ2hforHah/zTINRyie5006Jd2HKy2qz7snEJXl0KLfsJZW/wst9g6R2rFvqba3VpNYdu1hDcA== - prettier@1.16.3: version "1.16.3" resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.16.3.tgz#8c62168453badef702f34b45b6ee899574a6a65d" integrity sha512-kn/GU6SMRYPxUakNXhpP0EedT/KmaPzr0H5lIsDogrykbaxOpOfAFfk5XA7DZrJyMAv1wlMV3CPcZruGXVVUZw== +prettier@1.16.4: + version "1.16.4" + resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.16.4.tgz#73e37e73e018ad2db9c76742e2647e21790c9717" + integrity sha512-ZzWuos7TI5CKUeQAtFd6Zhm2s6EpAD/ZLApIhsF9pRvRtM1RFo61dM/4MSRUA0SuLugA/zgrZD8m0BaY46Og7g== + pretty-format@^24.0.0: version "24.0.0" resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-24.0.0.tgz#cb6599fd73ac088e37ed682f61291e4678f48591" |