diff --git a/.rubocop.yml b/.rubocop.yml index c84755859cbe9416d2c76c1b756ed8db5f7a59b5..bec2464c74025c969ca8fd31c0049471449f312c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -453,6 +453,10 @@ Style/VariableName: EnforcedStyle: snake_case Enabled: true +# Use the configured style when numbering variables. +Style/VariableNumber: + Enabled: false + # Use when x then ... for one-line cases. Style/WhenThen: Enabled: true diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b6ed6e333973ca63311a41234efc50f1dd8443df..11b34fafa2ae122534e8ba213e58467c89fab095 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,21 +1,21 @@ # This configuration was generated by # `rubocop --auto-gen-config --exclude-limit 0` -# on 2016-09-14 15:44:53 -0400 using RuboCop version 0.42.0. +# on 2016-10-04 13:16:20 +0200 using RuboCop version 0.43.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 158 +# Offense count: 160 Lint/AmbiguousRegexpLiteral: Enabled: false -# Offense count: 41 +# Offense count: 40 # Configuration parameters: AllowSafeAssignment. Lint/AssignmentInCondition: Enabled: false -# Offense count: 16 +# Offense count: 18 Lint/HandleExceptions: Enabled: false @@ -23,11 +23,21 @@ Lint/HandleExceptions: Lint/Loop: Enabled: false -# Offense count: 16 +# Offense count: 19 Lint/ShadowingOuterLocalVariable: Enabled: false -# Offense count: 49 +# Offense count: 9 +# Cop supports --auto-correct. +Lint/UnifiedInteger: + Enabled: false + +# Offense count: 13 +# Cop supports --auto-correct. +Lint/UnneededSplatExpansion: + Enabled: false + +# Offense count: 69 # Cop supports --auto-correct. # Configuration parameters: IgnoreEmptyBlocks, AllowUnusedKeywordArguments. Lint/UnusedBlockArgument: @@ -39,32 +49,81 @@ Lint/UnusedBlockArgument: Lint/UnusedMethodArgument: Enabled: false -# Offense count: 9 -# Cop supports --auto-correct. -Performance/PushSplat: - Enabled: false - # Offense count: 2 # Cop supports --auto-correct. Performance/RedundantBlockCall: Enabled: false -# Offense count: 4 +# Offense count: 5 # Cop supports --auto-correct. Performance/RedundantMatch: Enabled: false -# Offense count: 27 +# Offense count: 26 # Cop supports --auto-correct. # Configuration parameters: MaxKeyValuePairs. Performance/RedundantMerge: Enabled: false -# Offense count: 61 +# Offense count: 7 +RSpec/BeEql: + Enabled: false + +# Offense count: 20 +# Configuration parameters: CustomIncludeMethods. +RSpec/EmptyExampleGroup: + Enabled: false + +# Offense count: 16 +RSpec/ExpectActual: + Enabled: false + +# Offense count: 34 +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: implicit, each, example +RSpec/HookArgument: + Enabled: false + +# Offense count: 168 +RSpec/LeadingSubject: + Enabled: false + +# Offense count: 162 +RSpec/LetSetup: + Enabled: false + +# Offense count: 10 +RSpec/MessageChain: + Enabled: false + +# Offense count: 714 +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: allow, expect +RSpec/MessageExpectation: + Enabled: false + +# Offense count: 2423 +RSpec/MultipleExpectations: + Max: 36 + +# Offense count: 1504 +RSpec/NamedSubject: + Enabled: false + +# Offense count: 1335 +# Configuration parameters: MaxNesting. +RSpec/NestedGroups: + Enabled: false + +# Offense count: 99 +RSpec/SubjectStub: + Enabled: false + +# Offense count: 64 Rails/OutputSafety: Enabled: false -# Offense count: 129 +# Offense count: 151 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: strict, flexible Rails/TimeZone: @@ -77,58 +136,63 @@ Rails/TimeZone: Rails/Validation: Enabled: false -# Offense count: 273 +# Offense count: 2 +# Cop supports --auto-correct. +Security/JSONLoad: + Enabled: false + +# Offense count: 284 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: with_first_parameter, with_fixed_indentation Style/AlignParameters: Enabled: false -# Offense count: 30 +# Offense count: 28 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: always, conditionals Style/AndOr: Enabled: false -# Offense count: 50 +# Offense count: 52 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: percent_q, bare_percent Style/BarePercentLiterals: Enabled: false -# Offense count: 289 +# Offense count: 291 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: braces, no_braces, context_dependent Style/BracesAroundHashParameters: Enabled: false -# Offense count: 5 +# Offense count: 6 Style/CaseEquality: Enabled: false -# Offense count: 19 +# Offense count: 26 # Cop supports --auto-correct. Style/ColonMethodCall: Enabled: false -# Offense count: 3 +# Offense count: 2 # Cop supports --auto-correct. # Configuration parameters: Keywords. # Keywords: TODO, FIXME, OPTIMIZE, HACK, REVIEW Style/CommentAnnotation: Enabled: false -# Offense count: 33 +# Offense count: 30 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, SingleLineConditionsOnly. # SupportedStyles: assign_to_condition, assign_inside_condition Style/ConditionalAssignment: Enabled: false -# Offense count: 881 +# Offense count: 957 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: leading, trailing @@ -139,12 +203,12 @@ Style/DotPosition: Style/DoubleNegation: Enabled: false -# Offense count: 4 +# Offense count: 6 # Cop supports --auto-correct. Style/EachWithObject: Enabled: false -# Offense count: 25 +# Offense count: 26 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: empty, nil, both @@ -156,24 +220,24 @@ Style/EmptyElse: Style/EmptyLiteral: Enabled: false -# Offense count: 135 +# Offense count: 140 # Cop supports --auto-correct. # Configuration parameters: AllowForAlignment, ForceEqualSignAlignment. Style/ExtraSpacing: Enabled: false -# Offense count: 7 +# Offense count: 6 # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: format, sprintf, percent Style/FormatString: Enabled: false -# Offense count: 51 +# Offense count: 201 # Configuration parameters: MinBodyLength. Style/GuardClause: Enabled: false -# Offense count: 9 +# Offense count: 11 Style/IfInsideElse: Enabled: false @@ -183,21 +247,21 @@ Style/IfInsideElse: Style/IfUnlessModifier: Enabled: false -# Offense count: 52 +# Offense count: 53 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: special_inside_parentheses, consistent, align_brackets Style/IndentArray: Enabled: false -# Offense count: 97 +# Offense count: 95 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, IndentationWidth. # SupportedStyles: special_inside_parentheses, consistent, align_braces Style/IndentHash: Enabled: false -# Offense count: 12 +# Offense count: 29 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: line_count_dependent, lambda, literal @@ -209,7 +273,7 @@ Style/Lambda: Style/LineEndConcatenation: Enabled: false -# Offense count: 13 +# Offense count: 15 # Cop supports --auto-correct. Style/MethodCallParentheses: Enabled: false @@ -218,7 +282,7 @@ Style/MethodCallParentheses: Style/MethodMissing: Enabled: false -# Offense count: 85 +# Offense count: 95 # Cop supports --auto-correct. Style/MutableConstant: Enabled: false @@ -235,14 +299,14 @@ Style/NestedParenthesizedCalls: Style/Next: Enabled: false -# Offense count: 8 +# Offense count: 12 # Cop supports --auto-correct. # Configuration parameters: EnforcedOctalStyle, SupportedOctalStyles. # SupportedOctalStyles: zero_with_o, zero_only Style/NumericLiteralPrefix: Enabled: false -# Offense count: 64 +# Offense count: 53 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: predicate, comparison @@ -254,7 +318,7 @@ Style/NumericPredicate: Style/ParallelAssignment: Enabled: false -# Offense count: 264 +# Offense count: 294 # Cop supports --auto-correct. # Configuration parameters: PreferredDelimiters. Style/PercentLiteralDelimiters: @@ -272,7 +336,7 @@ Style/PercentQLiterals: Style/PerlBackrefs: Enabled: false -# Offense count: 35 +# Offense count: 38 # Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist. # NamePrefix: is_, has_, have_ # NamePrefixBlacklist: is_, has_, have_ @@ -280,7 +344,7 @@ Style/PerlBackrefs: Style/PredicateName: Enabled: false -# Offense count: 27 +# Offense count: 26 # Cop supports --auto-correct. Style/PreferredHashMethods: Enabled: false @@ -312,12 +376,12 @@ Style/RedundantException: Style/RedundantFreeze: Enabled: false -# Offense count: 408 +# Offense count: 427 # Cop supports --auto-correct. Style/RedundantSelf: Enabled: false -# Offense count: 93 +# Offense count: 97 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, AllowInnerSlashes. # SupportedStyles: slashes, percent_r, mixed @@ -329,7 +393,12 @@ Style/RegexpLiteral: Style/RescueModifier: Enabled: false -# Offense count: 5 +# Offense count: 114 +# Cop supports --auto-correct. +Style/SafeNavigation: + Enabled: false + +# Offense count: 7 # Cop supports --auto-correct. Style/SelfAssignment: Enabled: false @@ -346,7 +415,7 @@ Style/SingleLineBlockParams: Style/SingleLineMethods: Enabled: false -# Offense count: 124 +# Offense count: 125 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: space, no_space @@ -359,19 +428,19 @@ Style/SpaceBeforeBlockBraces: Style/SpaceBeforeFirstArg: Enabled: false -# Offense count: 141 +# Offense count: 145 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles, EnforcedStyleForEmptyBraces, SpaceBeforeBlockParameters. # SupportedStyles: space, no_space Style/SpaceInsideBlockBraces: Enabled: false -# Offense count: 96 +# Offense count: 99 # Cop supports --auto-correct. Style/SpaceInsideBrackets: Enabled: false -# Offense count: 62 +# Offense count: 65 # Cop supports --auto-correct. Style/SpaceInsideParens: Enabled: false @@ -381,21 +450,21 @@ Style/SpaceInsideParens: Style/SpaceInsidePercentLiteralDelimiters: Enabled: false -# Offense count: 40 +# Offense count: 41 # Cop supports --auto-correct. # Configuration parameters: SupportedStyles. # SupportedStyles: use_perl_names, use_english_names Style/SpecialGlobalVars: EnforcedStyle: use_perl_names -# Offense count: 30 +# Offense count: 31 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle, SupportedStyles. # SupportedStyles: single_quotes, double_quotes Style/StringLiteralsInInterpolation: Enabled: false -# Offense count: 32 +# Offense count: 33 # Cop supports --auto-correct. # Configuration parameters: IgnoredMethods. # IgnoredMethods: respond_to, define_method @@ -409,7 +478,7 @@ Style/SymbolProc: Style/TernaryParentheses: Enabled: false -# Offense count: 24 +# Offense count: 29 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyleForMultiline, SupportedStyles. # SupportedStyles: comma, consistent_comma, no_comma diff --git a/CHANGELOG b/CHANGELOG index dd3b15f513b4829a7bd44852ff8c38b6043253f7..d424fbe43b866a7ffe09a5243a198f89956cd5bb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.13.0 (unreleased) + - Update runner version only when updating contacted_at - Add link from system note to compare with previous version - Use gitlab-shell v3.6.2 (GIT TRACE logging) - Fix centering of custom header logos (Ashley Dumaine) @@ -15,12 +16,15 @@ v 8.13.0 (unreleased) - Simplify Mentionable concern instance methods - Fix permission for setting an issue's due date - Expose expires_at field when sharing project on API + - Fix VueJS template tags being rendered in code comments - Fix issue with page scrolling to top when closing or pinning sidebar (lukehowell) - Allow the Koding integration to be configured through the API - Added soft wrap button to repository file/blob editor - Add word-wrap to issue title on issue and milestone boards (ClemMakesApps) + - Fix todos page mobile viewport layout (ClemMakesApps) - Fix robots.txt disallowing access to groups starting with "s" (Matt Harrison) - Close open merge request without source project (Katarzyna Kobierska Ula Budziszewska) + - Fix that manual jobs would no longer block jobs in the next stage. !6604 - Add configurable email subject suffix (Fu Xu) - Use a ConnectionPool for Rails.cache on Sidekiq servers - Replace `alias_method_chain` with `Module#prepend` @@ -34,6 +38,7 @@ v 8.13.0 (unreleased) - Add missing values to linter !6276 (Katarzyna Kobierska Ula Budziszewska) - Fix Long commit messages overflow viewport in file tree - Revert avoid touching file system on Build#artifacts? + - Stop using a Redis lease when updating the project activity timestamp whenever a new event is created - Add broadcast messages and alerts below sub-nav - Better empty state for Groups view - Update ruby-prof to 0.16.2. !6026 (Elan Ruusamäe) @@ -43,6 +48,7 @@ v 8.13.0 (unreleased) - Optimize GitHub importing for speed and memory - API: expose pipeline data in builds API (!6502, Guilherme Salazar) - Notify the Merger about merge after successful build (Dimitris Karakasilis) + - Reduce queries needed to find users using their SSH keys when pushing commits - Fix broken repository 500 errors in project list - Close todos when accepting merge requests via the API !6486 (tonygambone) - Changed Slack service user referencing from full name to username (Sebastian Poxhofer) @@ -50,6 +56,8 @@ v 8.13.0 (unreleased) v 8.12.4 (unreleased) - Fix type mismatch bug when closing Jira issue + - Skip wiki creation when GitHub project has wiki enabled + - Fix failed project deletion when feature visibility set to private - Fix issues importing services via Import/Export - Restrict failed login attempts for users with 2FA enabled - Fix "Copy to clipboard" tooltip to say "Copied!" when clipboard button is clicked. (lukehowell) @@ -57,7 +65,7 @@ v 8.12.4 (unreleased) v 8.12.3 - Update Gitlab Shell to support low IO priority for storage moves -v 8.12.2 (unreleased) +v 8.12.2 - Fix Import/Export not recognising correctly the imported services. - Fix snippets pagination - Fix "Create project" button layout when visibility options are restricted diff --git a/Gemfile b/Gemfile index d54beedf67e93feeda7fa9d48a4ec4428d435ab9..18654a1e88c40dec244ea16181f1e47c32f2a8dc 100644 --- a/Gemfile +++ b/Gemfile @@ -295,7 +295,7 @@ group :development, :test do gem 'spring-commands-spinach', '~> 1.1.0' gem 'spring-commands-teaspoon', '~> 0.0.2' - gem 'rubocop', '~> 0.42.0', require: false + gem 'rubocop', '~> 0.43.0', require: false gem 'rubocop-rspec', '~> 1.5.0', require: false gem 'scss_lint', '~> 0.47.0', require: false gem 'haml_lint', '~> 0.18.2', require: false diff --git a/Gemfile.lock b/Gemfile.lock index 6f8a823686601eda96fdc61ea7cb63dec814395d..3f756fec929a26683b59c14935d8d008f88d7a21 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -487,7 +487,7 @@ GEM orm_adapter (0.5.0) paranoia (2.1.4) activerecord (~> 4.0) - parser (2.3.1.2) + parser (2.3.1.4) ast (~> 2.2) pg (0.18.4) pkg-config (1.1.7) @@ -620,7 +620,7 @@ GEM rspec-retry (0.4.5) rspec-core rspec-support (3.5.0) - rubocop (0.42.0) + rubocop (0.43.0) parser (>= 2.3.1.1, < 3.0) powerpack (~> 0.1) rainbow (>= 1.99.1, < 3.0) @@ -938,7 +938,7 @@ DEPENDENCIES rqrcode-rails3 (~> 0.1.7) rspec-rails (~> 3.5.0) rspec-retry (~> 0.4.5) - rubocop (~> 0.42.0) + rubocop (~> 0.43.0) rubocop-rspec (~> 1.5.0) ruby-fogbugz (~> 0.2.1) ruby-prof (~> 0.16.2) diff --git a/app/assets/stylesheets/pages/todos.scss b/app/assets/stylesheets/pages/todos.scss index 68a5d1ae06c6d8bbf147dff464fb84853f2a6385..ea76fe188763c6f26e113f0399ebf53dbac281ec 100644 --- a/app/assets/stylesheets/pages/todos.scss +++ b/app/assets/stylesheets/pages/todos.scss @@ -51,6 +51,7 @@ -webkit-flex-direction: column; flex-direction: column; margin-left: 10px; + min-width: 55px; } .todo-item { @@ -120,6 +121,14 @@ } } +@media (max-width: $screen-sm-max) { + .todos-filters { + .dropdown-menu-toggle { + width: 135px; + } + } +} + @media (max-width: $screen-xs-max) { .todo { .avatar { @@ -141,4 +150,14 @@ padding-left: 10px; } } + + .todos-filters { + .row-content-block { + padding-bottom: 50px; + } + + .dropdown-menu-toggle { + width: 100%; + } + } } diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 663c5b1e2315a3def61cb9672786eb2cdae8f490..97df74b0cfe8c695858bb60f93f16d1a60470e20 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -196,7 +196,7 @@ module Ci end def has_warnings? - builds.latest.ignored.any? + builds.latest.failed_but_allowed.any? end def config_processor diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index ed5d4b13b7eedd75730c5632cb26f0d212c503b0..44cb19ece3bf1f482e7759e8befbb4a8bc4dcc5a 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -2,7 +2,7 @@ module Ci class Runner < ActiveRecord::Base extend Ci::Model - LAST_CONTACT_TIME = 2.hours.ago + LAST_CONTACT_TIME = 1.hour.ago AVAILABLE_SCOPES = %w[specific shared active paused online] FORM_EDITABLE = %i[description tag_list active run_untagged locked] diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 736db1ab0f6da04e081bdd8b3324cd7cf98823c1..ee3396abe045c919538bed02c91fe4c9f2631235 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -24,7 +24,22 @@ class CommitStatus < ActiveRecord::Base scope :retried, -> { where.not(id: latest) } scope :ordered, -> { order(:name) } - scope :ignored, -> { where(allow_failure: true, status: [:failed, :canceled]) } + + scope :failed_but_allowed, -> do + where(allow_failure: true, status: [:failed, :canceled]) + end + + scope :exclude_ignored, -> do + quoted_when = connection.quote_column_name('when') + # We want to ignore failed_but_allowed jobs + where("allow_failure = ? OR status IN (?)", + false, all_state_names - [:failed, :canceled]). + # We want to ignore skipped manual jobs + where("#{quoted_when} <> ? OR status <> ?", 'manual', 'skipped'). + # We want to ignore skipped on_failure + where("#{quoted_when} <> ? OR status <> ?", 'on_failure', 'skipped') + end + scope :latest_ci_stages, -> { latest.ordered.includes(project: :namespace) } scope :retried_ci_stages, -> { retried.ordered.includes(project: :namespace) } @@ -111,7 +126,7 @@ class CommitStatus < ActiveRecord::Base end end - def ignored? + def failed_but_allowed? allow_failure? && (failed? || canceled?) end diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb index 0fa4df0fb5617b2161ef11845188105fa4c34612..9f64f76721d46463f29b3bb9b1b1a9109f83b378 100644 --- a/app/models/concerns/has_status.rb +++ b/app/models/concerns/has_status.rb @@ -8,32 +8,32 @@ module HasStatus class_methods do def status_sql - scope = all + scope = if respond_to?(:exclude_ignored) + exclude_ignored + else + all + end builds = scope.select('count(*)').to_sql created = scope.created.select('count(*)').to_sql success = scope.success.select('count(*)').to_sql - ignored = scope.ignored.select('count(*)').to_sql if scope.respond_to?(:ignored) - ignored ||= '0' pending = scope.pending.select('count(*)').to_sql running = scope.running.select('count(*)').to_sql - canceled = scope.canceled.select('count(*)').to_sql skipped = scope.skipped.select('count(*)').to_sql + canceled = scope.canceled.select('count(*)').to_sql - deduce_status = "(CASE + "(CASE + WHEN (#{builds})=(#{success}) THEN 'success' WHEN (#{builds})=(#{created}) THEN 'created' - WHEN (#{builds})=(#{skipped}) THEN 'skipped' - WHEN (#{builds})=(#{success})+(#{ignored})+(#{skipped}) THEN 'success' - WHEN (#{builds})=(#{created})+(#{pending})+(#{skipped}) THEN 'pending' - WHEN (#{builds})=(#{canceled})+(#{success})+(#{ignored})+(#{skipped}) THEN 'canceled' + WHEN (#{builds})=(#{success})+(#{skipped}) THEN 'skipped' + WHEN (#{builds})=(#{success})+(#{skipped})+(#{canceled}) THEN 'canceled' + WHEN (#{builds})=(#{created})+(#{skipped})+(#{pending}) THEN 'pending' WHEN (#{running})+(#{pending})+(#{created})>0 THEN 'running' ELSE 'failed' END)" - - deduce_status end def status - all.pluck(self.status_sql).first + all.pluck(status_sql).first end def started_at @@ -43,6 +43,10 @@ module HasStatus def finished_at all.maximum(:finished_at) end + + def all_state_names + state_machines.values.flat_map(&:states).flat_map { |s| s.map(&:name) } + end end included do diff --git a/app/models/event.rb b/app/models/event.rb index 55a76e26f3cacc61bd5cfd0093d0d095bb5e8af8..633019fe0af4bfd3f77e76bc29925d8451ecebb8 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -328,13 +328,15 @@ class Event < ActiveRecord::Base def reset_project_activity return unless project - # Don't even bother obtaining a lock if the last update happened less than - # 60 minutes ago. + # Don't bother updating if we know the project was updated recently. return if recent_update? - return unless try_obtain_lease - - project.update_column(:last_activity_at, created_at) + # At this point it's possible for multiple threads/processes to try to + # update the project. Only one query should actually perform the update, + # hence we add the extra WHERE clause for last_activity_at. + Project.unscoped.where(id: project_id). + where('last_activity_at > ?', RESET_PROJECT_ACTIVITY_INTERVAL.ago). + update_all(last_activity_at: created_at) end private @@ -342,11 +344,4 @@ class Event < ActiveRecord::Base def recent_update? project.last_activity_at > RESET_PROJECT_ACTIVITY_INTERVAL.ago end - - def try_obtain_lease - Gitlab::ExclusiveLease. - new("project:update_last_activity_at:#{project.id}", - timeout: RESET_PROJECT_ACTIVITY_INTERVAL.to_i). - try_obtain - end end diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index 8c9534c3565f95b80cb6a0b12ef47e40468c4949..530f7d5a30e6ad182e1aa1a0f68dfafc40c40c99 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -20,7 +20,10 @@ class ProjectFeature < ActiveRecord::Base FEATURES = %i(issues merge_requests wiki snippets builds) - belongs_to :project + # Default scopes force us to unscope here since a service may need to check + # permissions for a project in pending_delete + # http://stackoverflow.com/questions/1540645/how-to-disable-default-scope-for-a-belongs-to + belongs_to :project, -> { unscope(where: :pending_delete) } default_value_for :builds_access_level, value: ENABLED, allows_nil: false default_value_for :issues_access_level, value: ENABLED, allows_nil: false diff --git a/app/models/user.rb b/app/models/user.rb index 7f5a85629071eeb4dbd9aba3648df303121b0be2..508efd8505065935fe076f0d85e11ef44aa246ae 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -279,6 +279,11 @@ class User < ActiveRecord::Base find_by('users.username = ? OR users.id = ?', name_or_id.to_s, name_or_id.to_i) end + # Returns a user for the given SSH key. + def find_by_ssh_key_id(key_id) + find_by(id: Key.unscoped.select(:user_id).where(id: key_id)) + end + def build_user(attrs = {}) User.new(attrs) end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index be749ba4a1c3162621c59daba1a277dca9e6cc6c..76266139d09c22db9b17230f6f45347a2a679691 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -7,6 +7,8 @@ module Projects def execute forked_from_project_id = params.delete(:forked_from_project_id) import_data = params.delete(:import_data) + @skip_wiki = params.delete(:skip_wiki) + @project = Project.new(params) # Make sure that the user is allowed to use the specified visibility level @@ -80,7 +82,7 @@ module Projects log_info("#{@project.owner.name} created a new project \"#{@project.name_with_namespace}\"") unless @project.gitlab_project_import? - @project.create_wiki if @project.feature_available?(:wiki, current_user) + @project.create_wiki unless skip_wiki? @project.build_missing_services @project.create_labels @@ -94,6 +96,10 @@ module Projects end end + def skip_wiki? + !@project.feature_available?(:wiki, current_user) || @skip_wiki + end + def save_project_and_import_data(import_data) Project.transaction do @project.create_or_update_import_data(data: import_data[:data], credentials: import_data[:credentials]) if import_data diff --git a/doc/administration/housekeeping.md b/doc/administration/housekeeping.md index 34b4f1faa94adfc9e3cb1b4b75cc148590d0113c..ad1fa98b63ba2f4ef81d0fecb238b78d794afac9 100644 --- a/doc/administration/housekeeping.md +++ b/doc/administration/housekeeping.md @@ -12,7 +12,7 @@ revisions (to reduce disk space and increase performance) and removing unreachable objects which may have been created from prior invocations of `git add`. -You can find this option under your **[Project] > Settings**. +You can find this option under your **[Project] > Edit Project**. --- diff --git a/doc/administration/img/housekeeping_settings.png b/doc/administration/img/housekeeping_settings.png index f72ad9a45d5c16d31a63c3e4f8b4853b0d7dc281..6ebc6205635f81fcba372bd57b70c492f5c9b144 100644 Binary files a/doc/administration/img/housekeeping_settings.png and b/doc/administration/img/housekeeping_settings.png differ diff --git a/doc/ci/docker/using_docker_images.md b/doc/ci/docker/using_docker_images.md index a849905ac6b4f6a4341d8ba7f51fdca89a2dc544..520c8b36a959093639c17eb7eb763f4d4b9f7f58 100644 --- a/doc/ci/docker/using_docker_images.md +++ b/doc/ci/docker/using_docker_images.md @@ -221,7 +221,7 @@ time. *Note: The following commands are run without root privileges. You should be able to run docker with your regular user account.* -First start with creating a file named `build script`: +First start with creating a file named `build_script`: ```bash cat <<EOF > build_script diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index 2470362e01975c67d259f22e047332c60516d36f..af1e575fc89d6a9270c9e07a58a9eba7fda372b4 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -25,7 +25,7 @@ module Banzai return if customized?(whitelist[:transformers]) # Allow code highlighting - whitelist[:attributes]['pre'] = %w(class) + whitelist[:attributes]['pre'] = %w(class v-pre) whitelist[:attributes]['span'] = %w(class) # Allow table alignment diff --git a/lib/banzai/filter/syntax_highlight_filter.rb b/lib/banzai/filter/syntax_highlight_filter.rb index fcdb496aed245c4321c4ffded8dedfdf257a3129..026b81ac17549c1e4ca53fd3785555372b684036 100644 --- a/lib/banzai/filter/syntax_highlight_filter.rb +++ b/lib/banzai/filter/syntax_highlight_filter.rb @@ -30,7 +30,7 @@ module Banzai # users can still access an issue/comment/etc. end - highlighted = %(<pre class="#{css_classes}"><code>#{code}</code></pre>) + highlighted = %(<pre class="#{css_classes}" v-pre="true"><code>#{code}</code></pre>) # Extracted to a method to measure it replace_parent_pre_element(node, highlighted) diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 59f85416ee563a296f0b56e86c1a590ea1d20109..ed87a2603e842c504dc784dc07cd987c62db8a61 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -12,10 +12,9 @@ module Ci # POST /builds/register post "register" do authenticate_runner! - update_runner_last_contact(save: false) - update_runner_info required_attributes! [:token] not_found! unless current_runner.active? + update_runner_info build = Ci::RegisterBuildService.new.execute(current_runner) @@ -41,10 +40,11 @@ module Ci # PUT /builds/:id put ":id" do authenticate_runner! - update_runner_last_contact build = Ci::Build.where(runner_id: current_runner.id).running.find(params[:id]) forbidden!('Build has been erased!') if build.erased? + update_runner_info + build.update_attributes(trace: params[:trace]) if params[:trace] Gitlab::Metrics.add_event(:update_build, diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index 23353c6288572f3567e25e47552b27944bea5d56..e608f5f6cade3394fa7c085d982f541f35626cd6 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -3,7 +3,7 @@ module Ci module Helpers BUILD_TOKEN_HEADER = "HTTP_BUILD_TOKEN" BUILD_TOKEN_PARAM = :token - UPDATE_RUNNER_EVERY = 40 * 60 + UPDATE_RUNNER_EVERY = 10 * 60 def authenticate_runners! forbidden! unless runner_registration_token_valid? @@ -30,14 +30,22 @@ module Ci token && (build.valid_token?(token) || build.project.valid_runners_token?(token)) end - def update_runner_last_contact(save: true) - # Use a random threshold to prevent beating DB updates - # it generates a distribution between: [40m, 80m] + def update_runner_info + return unless update_runner? + + current_runner.contacted_at = Time.now + current_runner.assign_attributes(get_runner_version_from_params) + current_runner.save if current_runner.changed? + end + + def update_runner? + # Use a random threshold to prevent beating DB updates. + # It generates a distribution between [40m, 80m]. + # contacted_at_max_age = UPDATE_RUNNER_EVERY + Random.rand(UPDATE_RUNNER_EVERY) - if current_runner.contacted_at.nil? || Time.now - current_runner.contacted_at >= contacted_at_max_age - current_runner.contacted_at = Time.now - current_runner.save if current_runner.changed? && save - end + + current_runner.contacted_at.nil? || + (Time.now - current_runner.contacted_at) >= contacted_at_max_age end def build_not_found! @@ -57,11 +65,6 @@ module Ci attributes_for_keys(["name", "version", "revision", "platform", "architecture"], params["info"]) end - def update_runner_info - current_runner.assign_attributes(get_runner_version_from_params) - current_runner.save if current_runner.changed? - end - def max_artifacts_size current_application_settings.max_artifacts_size.megabytes.to_i end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index b83212444739702a90661020ee5e4971f73d6d23..4b70f33a851c22b905271d211701e5155e2c8bbb 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -170,10 +170,9 @@ module Gitlab end def import_wiki - unless project.wiki_enabled? + unless project.wiki.repository_exists? wiki = WikiFormatter.new(project) gitlab_shell.import_repository(project.repository_storage_path, wiki.path_with_namespace, wiki.import_url) - project.project.update_attribute(:wiki_access_level, ProjectFeature::ENABLED) end rescue Gitlab::Shell::Error => e # GitHub error message when the wiki repo has not been created, diff --git a/lib/gitlab/github_import/project_creator.rb b/lib/gitlab/github_import/project_creator.rb index 605abfabdabf8613b888b74faef3f95bdba45ebc..a241006884522f5fabc2a102c8dbbdf72195c26c 100644 --- a/lib/gitlab/github_import/project_creator.rb +++ b/lib/gitlab/github_import/project_creator.rb @@ -1,7 +1,7 @@ module Gitlab module GithubImport class ProjectCreator - attr_reader :repo, :namespace, :current_user, :session_data + attr_reader :repo, :name, :namespace, :current_user, :session_data def initialize(repo, name, namespace, current_user, session_data) @repo = repo @@ -12,24 +12,37 @@ module Gitlab end def execute - project = ::Projects::CreateService.new( + ::Projects::CreateService.new( current_user, - name: @name, - path: @name, + name: name, + path: name, description: repo.description, namespace_id: namespace.id, - visibility_level: repo.private ? Gitlab::VisibilityLevel::PRIVATE : ApplicationSetting.current.default_project_visibility, + visibility_level: visibility_level, import_type: "github", import_source: repo.full_name, - import_url: repo.clone_url.sub("https://", "https://#{@session_data[:github_access_token]}@") + import_url: import_url, + skip_wiki: skip_wiki ).execute + end + + private - # If repo has wiki we'll import it later - if repo.has_wiki? && project - project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED) - end + def import_url + repo.clone_url.sub('https://', "https://#{session_data[:github_access_token]}@") + end + + def visibility_level + repo.private ? Gitlab::VisibilityLevel::PRIVATE : ApplicationSetting.current.default_project_visibility + end - project + # + # If the GitHub project repository has wiki, we should not create the + # default wiki. Otherwise the GitHub importer will fail because the wiki + # repository already exist. + # + def skip_wiki + repo.has_wiki? end end end diff --git a/lib/gitlab/identifier.rb b/lib/gitlab/identifier.rb index 3e5d728f3bce43ecc99e37db99895f17ee7c0f74..f8809db21aa9a3b3e4a9348c69cdcbda464462b4 100644 --- a/lib/gitlab/identifier.rb +++ b/lib/gitlab/identifier.rb @@ -5,19 +5,61 @@ module Gitlab def identify(identifier, project, newrev) if identifier.blank? # Local push from gitlab - email = project.commit(newrev).author_email rescue nil - User.find_by(email: email) if email - + identify_using_commit(project, newrev) elsif identifier =~ /\Auser-\d+\Z/ # git push over http - user_id = identifier.gsub("user-", "") - User.find_by(id: user_id) - + identify_using_user(identifier) elsif identifier =~ /\Akey-\d+\Z/ # git push over ssh - key_id = identifier.gsub("key-", "") - Key.find_by(id: key_id).try(:user) + identify_using_ssh_key(identifier) + end + end + + # Tries to identify a user based on a commit SHA. + def identify_using_commit(project, ref) + commit = project.commit(ref) + + return if !commit || !commit.author_email + + email = commit.author_email + + identify_with_cache(:email, email) do + User.find_by(email: email) end end + + # Tries to identify a user based on a user identifier (e.g. "user-123"). + def identify_using_user(identifier) + user_id = identifier.gsub("user-", "") + + identify_with_cache(:user, user_id) do + User.find_by(id: user_id) + end + end + + # Tries to identify a user based on an SSH key identifier (e.g. "key-123"). + def identify_using_ssh_key(identifier) + key_id = identifier.gsub("key-", "") + + identify_with_cache(:ssh_key, key_id) do + User.find_by_ssh_key_id(key_id) + end + end + + def identify_with_cache(category, key) + if identification_cache[category].key?(key) + identification_cache[category][key] + else + identification_cache[category][key] = yield + end + end + + def identification_cache + @identification_cache ||= { + email: {}, + user: {}, + ssh_key: {} + } + end end end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index f1c522155d36d254ee98af97c12d94fad53866c4..5d7247e2a626bc3808abd2317c04394279ff6223 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -240,6 +240,18 @@ describe 'Comments', feature: true do is_expected.to have_css('.notes_holder .note', count: 1) is_expected.to have_button('Reply...') end + + it 'adds code to discussion' do + click_button 'Reply...' + + page.within(first('.js-discussion-note-form')) do + fill_in 'note[note]', with: '```{{ test }}```' + + click_button('Comment') + end + + expect(page).to have_content('{{ test }}') + end end end end diff --git a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb index b1370bca8332dfe2fbe95fddb2a89c7e7a8fad2b..d265d29ee86990066e43a29b2c23ce551e854480 100644 --- a/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb +++ b/spec/lib/banzai/filter/syntax_highlight_filter_spec.rb @@ -6,21 +6,21 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do context "when no language is specified" do it "highlights as plaintext" do result = filter('<pre><code>def fun end</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext"><code>def fun end</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" v-pre="true"><code>def fun end</code></pre>') end end context "when a valid language is specified" do it "highlights as that language" do result = filter('<pre><code class="ruby">def fun end</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby"><code><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight ruby" v-pre="true"><code><span class="k">def</span> <span class="nf">fun</span> <span class="k">end</span></code></pre>') end end context "when an invalid language is specified" do it "highlights as plaintext" do result = filter('<pre><code class="gnuplot">This is a test</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext"><code>This is a test</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight js-syntax-highlight plaintext" v-pre="true"><code>This is a test</code></pre>') end end @@ -31,7 +31,7 @@ describe Banzai::Filter::SyntaxHighlightFilter, lib: true do it "highlights as plaintext" do result = filter('<pre><code class="ruby">This is a test</code></pre>') - expect(result.to_html).to eq('<pre class="code highlight"><code>This is a test</code></pre>') + expect(result.to_html).to eq('<pre class="code highlight" v-pre="true"><code>This is a test</code></pre>') end end end diff --git a/spec/lib/gitlab/github_import/project_creator_spec.rb b/spec/lib/gitlab/github_import/project_creator_spec.rb index ab06b7bc5bb295cf008a7acef1e1ded032e5b4bc..a73b1f4ff5d191bea63831e5a4231af3fe70d6a0 100644 --- a/spec/lib/gitlab/github_import/project_creator_spec.rb +++ b/spec/lib/gitlab/github_import/project_creator_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do expect(project.import_data.credentials).to eq(user: 'asdffg', password: nil) end - context 'when Github project is private' do + context 'when GitHub project is private' do it 'sets project visibility to private' do repo.private = true @@ -43,7 +43,7 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do end end - context 'when Github project is public' do + context 'when GitHub project is public' do before do allow_any_instance_of(ApplicationSetting).to receive(:default_project_visibility).and_return(Gitlab::VisibilityLevel::INTERNAL) end @@ -56,5 +56,25 @@ describe Gitlab::GithubImport::ProjectCreator, lib: true do expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) end end + + context 'when GitHub project has wiki' do + it 'does not create the wiki repository' do + allow(repo).to receive(:has_wiki?).and_return(true) + + project = service.execute + + expect(project.wiki.repository_exists?).to eq false + end + end + + context 'when GitHub project does not have wiki' do + it 'creates the wiki repository' do + allow(repo).to receive(:has_wiki?).and_return(false) + + project = service.execute + + expect(project.wiki.repository_exists?).to eq true + end + end end end diff --git a/spec/lib/gitlab/identifier_spec.rb b/spec/lib/gitlab/identifier_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..47d6f1007d1b8b2b8daae451c2e55ef218ee8e5b --- /dev/null +++ b/spec/lib/gitlab/identifier_spec.rb @@ -0,0 +1,123 @@ +require 'spec_helper' + +describe Gitlab::Identifier do + let(:identifier) do + Class.new { include Gitlab::Identifier }.new + end + + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let(:key) { create(:key, user: user) } + + describe '#identify' do + context 'without an identifier' do + it 'identifies the user using a commit' do + expect(identifier).to receive(:identify_using_commit). + with(project, '123') + + identifier.identify('', project, '123') + end + end + + context 'with a user identifier' do + it 'identifies the user using a user ID' do + expect(identifier).to receive(:identify_using_user). + with("user-#{user.id}") + + identifier.identify("user-#{user.id}", project, '123') + end + end + + context 'with an SSH key identifier' do + it 'identifies the user using an SSH key ID' do + expect(identifier).to receive(:identify_using_ssh_key). + with("key-#{key.id}") + + identifier.identify("key-#{key.id}", project, '123') + end + end + end + + describe '#identify_using_commit' do + it "returns the User for an existing commit author's Email address" do + commit = double(:commit, author_email: user.email) + + expect(project).to receive(:commit).with('123').and_return(commit) + + expect(identifier.identify_using_commit(project, '123')).to eq(user) + end + + it 'returns nil when no user could be found' do + allow(project).to receive(:commit).with('123').and_return(nil) + + expect(identifier.identify_using_commit(project, '123')).to be_nil + end + + it 'returns nil when the commit does not have an author Email' do + commit = double(:commit, author_email: nil) + + expect(project).to receive(:commit).with('123').and_return(commit) + + expect(identifier.identify_using_commit(project, '123')).to be_nil + end + + it 'caches the found users per Email' do + commit = double(:commit, author_email: user.email) + + expect(project).to receive(:commit).with('123').twice.and_return(commit) + expect(User).to receive(:find_by).once.and_call_original + + 2.times do + expect(identifier.identify_using_commit(project, '123')).to eq(user) + end + end + end + + describe '#identify_using_user' do + it 'returns the User for an existing ID in the identifier' do + found = identifier.identify_using_user("user-#{user.id}") + + expect(found).to eq(user) + end + + it 'returns nil for a non existing user ID' do + found = identifier.identify_using_user('user--1') + + expect(found).to be_nil + end + + it 'caches the found users per ID' do + expect(User).to receive(:find_by).once.and_call_original + + 2.times do + found = identifier.identify_using_user("user-#{user.id}") + + expect(found).to eq(user) + end + end + end + + describe '#identify_using_ssh_key' do + it 'returns the User for an existing SSH key' do + found = identifier.identify_using_ssh_key("key-#{key.id}") + + expect(found).to eq(user) + end + + it 'returns nil for an invalid SSH key' do + found = identifier.identify_using_ssh_key('key--1') + + expect(found).to be_nil + end + + it 'caches the found users per key' do + expect(User).to receive(:find_by_ssh_key_id).once.and_call_original + + 2.times do + found = identifier.identify_using_ssh_key("key-#{key.id}") + + expect(found).to eq(user) + end + end + end +end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index e7864b7ad33fbf0f50d440002a18e77e2093c46b..ae185de9ca379311cdcbc0d931e84c4f4c3524d1 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -39,8 +39,8 @@ describe Ci::Build, models: true do end end - describe '#ignored?' do - subject { build.ignored? } + describe '#failed_but_allowed?' do + subject { build.failed_but_allowed? } context 'when build is not allowed to fail' do before do diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 2f1baff5d66a9c83a5a29c25db8bc2dd8094e378..80c2a1bc7a987a32ca917c705dbf6724b6544010 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -7,7 +7,11 @@ describe CommitStatus, models: true do create(:ci_pipeline, project: project, sha: project.commit.id) end - let(:commit_status) { create(:commit_status, pipeline: pipeline) } + let(:commit_status) { create_status } + + def create_status(args = {}) + create(:commit_status, args.merge(pipeline: pipeline)) + end it { is_expected.to belong_to(:pipeline) } it { is_expected.to belong_to(:user) } @@ -125,32 +129,53 @@ describe CommitStatus, models: true do describe '.latest' do subject { CommitStatus.latest.order(:id) } - before do - @commit1 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'running' - @commit2 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'cc', status: 'pending' - @commit3 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'cc', status: 'success' - @commit4 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'bb', status: 'success' - @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'success' + let(:statuses) do + [create_status(name: 'aa', ref: 'bb', status: 'running'), + create_status(name: 'cc', ref: 'cc', status: 'pending'), + create_status(name: 'aa', ref: 'cc', status: 'success'), + create_status(name: 'cc', ref: 'bb', status: 'success'), + create_status(name: 'aa', ref: 'bb', status: 'success')] end it 'returns unique statuses' do - is_expected.to eq([@commit4, @commit5]) + is_expected.to eq(statuses.values_at(3, 4)) end end describe '.running_or_pending' do subject { CommitStatus.running_or_pending.order(:id) } - before do - @commit1 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'running' - @commit2 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'cc', status: 'pending' - @commit3 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: nil, status: 'success' - @commit4 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'dd', ref: nil, status: 'failed' - @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'ee', ref: nil, status: 'canceled' + let(:statuses) do + [create_status(name: 'aa', ref: 'bb', status: 'running'), + create_status(name: 'cc', ref: 'cc', status: 'pending'), + create_status(name: 'aa', ref: nil, status: 'success'), + create_status(name: 'dd', ref: nil, status: 'failed'), + create_status(name: 'ee', ref: nil, status: 'canceled')] end it 'returns statuses that are running or pending' do - is_expected.to eq([@commit1, @commit2]) + is_expected.to eq(statuses.values_at(0, 1)) + end + end + + describe '.exclude_ignored' do + subject { CommitStatus.exclude_ignored.order(:id) } + + let(:statuses) do + [create_status(when: 'manual', status: 'skipped'), + create_status(when: 'manual', status: 'success'), + create_status(when: 'manual', status: 'failed'), + create_status(when: 'on_failure', status: 'skipped'), + create_status(when: 'on_failure', status: 'success'), + create_status(when: 'on_failure', status: 'failed'), + create_status(allow_failure: true, status: 'success'), + create_status(allow_failure: true, status: 'failed'), + create_status(allow_failure: false, status: 'success'), + create_status(allow_failure: false, status: 'failed')] + end + + it 'returns statuses without what we want to ignore' do + is_expected.to eq(statuses.values_at(1, 2, 4, 5, 6, 8, 9)) end end diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index e118432d0987835800a87ccf6a3ea1d720ae6638..87bffbdc54e599d6fdbb694640e71358b277aa6c 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -1,26 +1,17 @@ require 'spec_helper' describe HasStatus do - before do - @object = Object.new - @object.extend(HasStatus::ClassMethods) - end - describe '.status' do - before do - allow(@object).to receive(:all).and_return(CommitStatus.where(id: statuses)) - end - - subject { @object.status } + subject { CommitStatus.status } shared_examples 'build status summary' do context 'all successful' do - let(:statuses) { Array.new(2) { create(type, status: :success) } } + let!(:statuses) { Array.new(2) { create(type, status: :success) } } it { is_expected.to eq 'success' } end context 'at least one failed' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :failed)] end @@ -28,7 +19,7 @@ describe HasStatus do end context 'at least one running' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :running)] end @@ -36,7 +27,7 @@ describe HasStatus do end context 'at least one pending' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :pending)] end @@ -44,7 +35,7 @@ describe HasStatus do end context 'success and failed but allowed to fail' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :failed, allow_failure: true)] end @@ -53,12 +44,15 @@ describe HasStatus do end context 'one failed but allowed to fail' do - let(:statuses) { [create(type, status: :failed, allow_failure: true)] } + let!(:statuses) do + [create(type, status: :failed, allow_failure: true)] + end + it { is_expected.to eq 'success' } end context 'success and canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :canceled)] end @@ -66,7 +60,7 @@ describe HasStatus do end context 'one failed and one canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :failed), create(type, status: :canceled)] end @@ -74,7 +68,7 @@ describe HasStatus do end context 'one failed but allowed to fail and one canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :failed, allow_failure: true), create(type, status: :canceled)] end @@ -83,7 +77,7 @@ describe HasStatus do end context 'one running one canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :running), create(type, status: :canceled)] end @@ -91,14 +85,15 @@ describe HasStatus do end context 'all canceled' do - let(:statuses) do + let!(:statuses) do [create(type, status: :canceled), create(type, status: :canceled)] end + it { is_expected.to eq 'canceled' } end context 'success and canceled but allowed to fail' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :canceled, allow_failure: true)] end @@ -107,7 +102,7 @@ describe HasStatus do end context 'one finished and second running but allowed to fail' do - let(:statuses) do + let!(:statuses) do [create(type, status: :success), create(type, status: :running, allow_failure: true)] end @@ -118,11 +113,13 @@ describe HasStatus do context 'ci build statuses' do let(:type) { :ci_build } + it_behaves_like 'build status summary' end context 'generic commit statuses' do let(:type) { :generic_commit_status } + it_behaves_like 'build status summary' end end diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 8600eb4d2c40d7e1ec443f260da53654e60a953d..af5002487cc72eaf35417f2dd1d14d293c8401cf 100644 --- a/spec/models/event_spec.rb +++ b/spec/models/event_spec.rb @@ -173,13 +173,11 @@ describe Event, models: true do it 'updates the project' do project.update(last_activity_at: 1.year.ago) - expect_any_instance_of(Gitlab::ExclusiveLease). - to receive(:try_obtain).and_return(true) + create_event(project, project.owner) - expect(project).to receive(:update_column). - with(:last_activity_at, a_kind_of(Time)) + project.reload - create_event(project, project.owner) + project.last_activity_at <= 1.minute.ago end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ef854a2532153201d191be13c7827c9fdca8b0ec..3ab5ac78bba20f1899054e06d01fc6003ed8f6c5 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -308,8 +308,7 @@ describe Project, models: true do end describe 'last_activity methods' do - let(:timestamp) { Time.now - 2.hours } - let(:project) { create(:project, created_at: timestamp, updated_at: timestamp) } + let(:project) { create(:project, last_activity_at: 2.hours.ago) } describe 'last_activity' do it 'alias last_activity to last_event' do @@ -321,7 +320,6 @@ describe Project, models: true do describe 'last_activity_date' do it 'returns the creation date of the project\'s last event if present' do - expect_any_instance_of(Event).to receive(:try_obtain_lease).and_return(true) new_event = create(:event, project: project, created_at: Time.now) expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a1770d96f8319799859b8fad82602c718eb0b8b6..65b2896930a45eb0ce5e4a1ec94f319fa6ff6e89 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -610,6 +610,23 @@ describe User, models: true do end end + describe '.find_by_ssh_key_id' do + context 'using an existing SSH key ID' do + let(:user) { create(:user) } + let(:key) { create(:key, user: user) } + + it 'returns the corresponding User' do + expect(described_class.find_by_ssh_key_id(key.id)).to eq(user) + end + end + + context 'using an invalid SSH key ID' do + it 'returns nil' do + expect(described_class.find_by_ssh_key_id(-1)).to be_nil + end + end + end + describe '.by_login' do let(:username) { 'John' } let!(:user) { create(:user, username: username) } diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb index df97f1bf7b6a0e8b52546a3f7b6b1eb08caf89c3..7b7d62feb2c8c4bbd5b67aa10e340b7a2c93fae1 100644 --- a/spec/requests/ci/api/builds_spec.rb +++ b/spec/requests/ci/api/builds_spec.rb @@ -35,18 +35,24 @@ describe Ci::API::API do end end - it "starts a build" do - register_builds info: { platform: :darwin } - - expect(response).to have_http_status(201) - expect(json_response['sha']).to eq(build.sha) - expect(runner.reload.platform).to eq("darwin") - expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) - expect(json_response["variables"]).to include( - { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, - { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, - { "key" => "DB_NAME", "value" => "postgres", "public" => true } - ) + context 'when there is a pending build' do + it 'starts a build' do + register_builds info: { platform: :darwin } + + expect(response).to have_http_status(201) + expect(json_response['sha']).to eq(build.sha) + expect(runner.reload.platform).to eq("darwin") + expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) + expect(json_response["variables"]).to include( + { "key" => "CI_BUILD_NAME", "value" => "spinach", "public" => true }, + { "key" => "CI_BUILD_STAGE", "value" => "test", "public" => true }, + { "key" => "DB_NAME", "value" => "postgres", "public" => true } + ) + end + + it 'updates runner info' do + expect { register_builds }.to change { runner.reload.contacted_at } + end end context 'when builds are finished' do @@ -159,13 +165,18 @@ describe Ci::API::API do end context 'when runner is paused' do - let(:inactive_runner) { create(:ci_runner, :inactive, token: "InactiveRunner") } + let(:runner) { create(:ci_runner, :inactive, token: 'InactiveRunner') } - before do - register_builds inactive_runner.token + it 'responds with 404' do + register_builds + + expect(response).to have_http_status 404 end - it { expect(response).to have_http_status 404 } + it 'does not update runner info' do + expect { register_builds } + .not_to change { runner.reload.contacted_at } + end end def register_builds(token = runner.token, **params) diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb index 8326e5cd3139f07a959b4b97de72651f5bbdd264..ff113efd916befea2ec0f05da4582085e88a745d 100644 --- a/spec/services/ci/process_pipeline_service_spec.rb +++ b/spec/services/ci/process_pipeline_service_spec.rb @@ -18,7 +18,7 @@ describe Ci::ProcessPipelineService, services: true do all_builds.where.not(status: [:created, :skipped]) end - def create_builds + def process_pipeline described_class.new(pipeline.project, user).execute(pipeline) end @@ -36,26 +36,26 @@ describe Ci::ProcessPipelineService, services: true do end it 'processes a pipeline' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy succeed_pending expect(builds.success.count).to eq(2) - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy succeed_pending expect(builds.success.count).to eq(4) - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy succeed_pending expect(builds.success.count).to eq(5) - expect(create_builds).to be_falsey + expect(process_pipeline).to be_falsey end it 'does not process pipeline if existing stage is running' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pending.count).to eq(2) - expect(create_builds).to be_falsey + expect(process_pipeline).to be_falsey expect(builds.pending.count).to eq(2) end end @@ -67,7 +67,7 @@ describe Ci::ProcessPipelineService, services: true do end it 'automatically triggers a next stage when build finishes' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:drop) @@ -88,7 +88,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when builds are successful' do it 'properly creates builds' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -113,7 +113,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when test job fails' do it 'properly creates builds' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -138,7 +138,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when test and test_failure jobs fail' do it 'properly creates builds' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -164,7 +164,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when deploy job fails' do it 'properly creates builds' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -189,7 +189,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when build is canceled in the second stage' do it 'does not schedule builds after build has been canceled' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build') expect(builds.pluck(:status)).to contain_exactly('pending') pipeline.builds.running_or_pending.each(&:success) @@ -208,7 +208,7 @@ describe Ci::ProcessPipelineService, services: true do context 'when listing manual actions' do it 'returns only for skipped builds' do # currently all builds are created - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(manual_actions).to be_empty # succeed stage build @@ -230,6 +230,69 @@ describe Ci::ProcessPipelineService, services: true do end end + context 'when there are manual/on_failure jobs in earlier stages' do + before do + builds + process_pipeline + builds.each(&:reload) + end + + context 'when first stage has only manual jobs' do + let(:builds) do + [create_build('build', 0, 'manual'), + create_build('check', 1), + create_build('test', 2)] + end + + it 'starts from the second stage' do + expect(builds.map(&:status)).to eq(%w[skipped pending created]) + end + end + + context 'when second stage has only manual jobs' do + let(:builds) do + [create_build('check', 0), + create_build('build', 1, 'manual'), + create_build('test', 2)] + end + + it 'skips second stage and continues on third stage' do + expect(builds.map(&:status)).to eq(%w[pending created created]) + + builds.first.success + builds.each(&:reload) + + expect(builds.map(&:status)).to eq(%w[success skipped pending]) + end + end + + context 'when second stage has only on_failure jobs' do + let(:builds) do + [create_build('check', 0), + create_build('build', 1, 'on_failure'), + create_build('test', 2)] + end + + it 'skips second stage and continues on third stage' do + expect(builds.map(&:status)).to eq(%w[pending created created]) + + builds.first.success + builds.each(&:reload) + + expect(builds.map(&:status)).to eq(%w[success skipped pending]) + end + end + + def create_build(name, stage_idx, when_value = nil) + create(:ci_build, + :created, + pipeline: pipeline, + name: name, + stage_idx: stage_idx, + when: when_value) + end + end + context 'when failed build in the middle stage is retried' do context 'when failed build is the only unsuccessful build in the stage' do before do @@ -242,7 +305,7 @@ describe Ci::ProcessPipelineService, services: true do end it 'does trigger builds in the next stage' do - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pluck(:name)).to contain_exactly('build:1', 'build:2') pipeline.builds.running_or_pending.each(&:success) @@ -297,14 +360,14 @@ describe Ci::ProcessPipelineService, services: true do expect(all_builds.count).to eq(2) # Create builds will mark the created as pending - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.count).to eq(2) expect(all_builds.count).to eq(2) # When we builds succeed we will create a rest of pipeline from .gitlab-ci.yml # We will have 2 succeeded, 2 pending (from stage test), total 5 (one more build from deploy) succeed_pending - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.success.count).to eq(2) expect(builds.pending.count).to eq(2) expect(all_builds.count).to eq(5) @@ -312,14 +375,14 @@ describe Ci::ProcessPipelineService, services: true do # When we succeed the 2 pending from stage test, # We will queue a deploy stage, no new builds will be created succeed_pending - expect(create_builds).to be_truthy + expect(process_pipeline).to be_truthy expect(builds.pending.count).to eq(1) expect(builds.success.count).to eq(4) expect(all_builds.count).to eq(5) # When we succeed last pending build, we will have a total of 5 succeeded builds, no new builds will be created succeed_pending - expect(create_builds).to be_falsey + expect(process_pipeline).to be_falsey expect(builds.success.count).to eq(5) expect(all_builds.count).to eq(5) end diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 29341c5e57e129e4c023a0d656237586caa56b72..7dcd03496bbcae7df6be43437d632d175e260041 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -5,6 +5,7 @@ describe Projects::DestroyService, services: true do let!(:project) { create(:project, namespace: user.namespace) } let!(:path) { project.repository.path_to_repo } let!(:remove_path) { path.sub(/\.git\Z/, "+#{project.id}+deleted.git") } + let!(:async) { false } # execute or async_execute context 'Sidekiq inline' do before do @@ -28,6 +29,22 @@ describe Projects::DestroyService, services: true do it { expect(Dir.exist?(remove_path)).to be_truthy } end + context 'async delete of project with private issue visibility' do + let!(:async) { true } + + before do + project.project_feature.update_attribute("issues_access_level", ProjectFeature::PRIVATE) + # Run sidekiq immediately to check that renamed repository will be removed + Sidekiq::Testing.inline! { destroy_project(project, user, {}) } + end + + it 'deletes the project' do + expect(Project.all).not_to include(project) + expect(Dir.exist?(path)).to be_falsey + expect(Dir.exist?(remove_path)).to be_falsey + end + end + context 'container registry' do before do stub_container_registry_config(enabled: true) @@ -52,6 +69,10 @@ describe Projects::DestroyService, services: true do end def destroy_project(project, user, params) - Projects::DestroyService.new(project, user, params).execute + if async + Projects::DestroyService.new(project, user, params).async_execute + else + Projects::DestroyService.new(project, user, params).execute + end end end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 1d2cf7acddd2ca6dd3bd9aa6465786a62fa71c65..ffeaafe654a893272ad174328cd6493e23124a99 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -79,7 +79,9 @@ describe PostReceive do end it "does not run if the author is not in the project" do - allow(Key).to receive(:find_by).with(hash_including(id: anything())) { nil } + allow_any_instance_of(Gitlab::GitPostReceive). + to receive(:identify_using_ssh_key). + and_return(nil) expect(project).not_to receive(:execute_hooks)