From 1bb7fc2fd6cc73a53636b80189bd7f66983967a4 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 26 Sep 2024 09:14:07 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- app/assets/stylesheets/framework/header.scss | 14 +-- app/models/label_note.rb | 4 +- app/models/namespace/traversal_hierarchy.rb | 31 +++-- .../17-5-ci-job-token-outbound-removal.yml | 38 ++++++ doc/api/protected_branches.md | 2 +- doc/ci/runners/configure_runners.md | 2 +- doc/update/deprecations.md | 16 +++ doc/user/compliance/audit_event_types.md | 2 +- lib/gitlab/database/transaction/settings.rb | 48 ++++++++ lib/gitlab/query_limiting/transaction.rb | 4 + .../database/transaction/settings_spec.rb | 111 ++++++++++++++++++ .../gitlab/query_limiting/transaction_spec.rb | 2 + .../namespace/traversal_hierarchy_spec.rb | 19 +-- .../models/label_note_shared_examples.rb | 31 ++--- .../row_lock_shared_examples.rb | 8 +- 15 files changed, 283 insertions(+), 49 deletions(-) create mode 100644 data/deprecations/17-5-ci-job-token-outbound-removal.yml create mode 100644 lib/gitlab/database/transaction/settings.rb create mode 100644 spec/lib/gitlab/database/transaction/settings_spec.rb diff --git a/app/assets/stylesheets/framework/header.scss b/app/assets/stylesheets/framework/header.scss index fdf16eb1705..d47dbb74627 100644 --- a/app/assets/stylesheets/framework/header.scss +++ b/app/assets/stylesheets/framework/header.scss @@ -67,7 +67,7 @@ top: $calc-system-headers-height; left: 0; right: 0; - background-color: $brand-charcoal; + background-color: var(--gl-color-brand-charcoal); @media (forced-colors: active) { border-bottom: 1px solid; @@ -90,7 +90,7 @@ &:hover, &:focus { - background-color: $brand-gray-04; + background-color: var(--gl-color-brand-gray-04); } &:focus, @@ -99,7 +99,7 @@ } &:active { - background-color: $brand-gray-03; + background-color: var(--gl-color-brand-gray-03); } img { @@ -132,19 +132,19 @@ padding: 6px 8px; height: 32px; border-radius: $gl-border-radius-base; - color: $gray-100; + color: var(--gl-color-neutral-100); font-weight: $gl-font-weight-normal; font-size: $gl-font-size; &:hover, &:focus, &:active { - color: $white; + color: var(--gl-color-neutral-0); } &:hover, &:focus { - background-color: $brand-gray-04; + background-color: var(--gl-color-brand-gray-04); text-decoration: none; } @@ -154,7 +154,7 @@ } &:active { - background-color: $brand-gray-03; + background-color: var(--gl-color-brand-gray-03); } } } diff --git a/app/models/label_note.rb b/app/models/label_note.rb index 8786a035eee..c711608e96a 100644 --- a/app/models/label_note.rb +++ b/app/models/label_note.rb @@ -16,7 +16,7 @@ class LabelNote < SyntheticNote resource ||= events.first.issuable label_note = from_event(events.first, resource: resource, resource_parent: resource_parent) - label_note.events = events.sort_by(&:id) + label_note.events = events.sort_by { |e| e.label&.name.to_s } label_note end @@ -59,7 +59,7 @@ class LabelNote < SyntheticNote # added 3 deleted labels # added ~1 ~2 labels def labels_str(label_refs, prefix: '') - existing_refs = label_refs.select { |ref| ref.present? }.sort + existing_refs = label_refs.select(&:present?) refs_str = existing_refs.empty? ? nil : existing_refs.join(' ') deleted = label_refs.count - existing_refs.count diff --git a/app/models/namespace/traversal_hierarchy.rb b/app/models/namespace/traversal_hierarchy.rb index 66f40fadf05..0f5d143a529 100644 --- a/app/models/namespace/traversal_hierarchy.rb +++ b/app/models/namespace/traversal_hierarchy.rb @@ -13,6 +13,10 @@ # class Namespace class TraversalHierarchy + include Transactions + + LOCK_TIMEOUT = '500ms' + attr_accessor :root def self.for_namespace(namespace) @@ -26,6 +30,7 @@ class Namespace end # Update all traversal_ids in the current namespace hierarchy. + # rubocop:disable Database/RescueQueryCanceled -- Measuring specific query timeouts def sync_traversal_ids! # An issue in Rails since 2013 prevents this kind of join based update in # ActiveRecord. https://github.com/rails/rails/issues/13496 @@ -46,17 +51,20 @@ class Namespace %w[namespaces], url: 'https://gitlab.com/gitlab-org/gitlab/-/issues/424279' ) do Namespace.transaction do - lock_options = 'FOR NO KEY UPDATE' - lock_options += ' NOWAIT' if Feature.enabled?(:sync_traversal_ids_nowait, Feature.current_request) - - @root.lock!(lock_options) + if sync_traversal_ids_nowait? + Gitlab::Database::Transaction::Settings.with('LOCK_TIMEOUT', LOCK_TIMEOUT) do + @root.lock!('FOR NO KEY UPDATE') + end + else + @root.lock!('FOR NO KEY UPDATE') + end Namespace.connection.exec_query(sql) end end - rescue ActiveRecord::LockWaitTimeout => e - if e.message.starts_with? 'PG::LockNotAvailable' - db_nowait_counter.increment(source: 'Namespace#sync_traversal_ids!') + rescue ActiveRecord::QueryCanceled => e + if e.message.include?("canceling statement due to statement timeout") + db_query_timeout_counter.increment(source: 'Namespace#sync_traversal_ids!') end raise @@ -71,6 +79,7 @@ class Namespace .joins("INNER JOIN (#{recursive_traversal_ids}) as cte ON namespaces.id = cte.id") .where('namespaces.traversal_ids::bigint[] <> cte.traversal_ids') end + # rubocop:enable Database/RescueQueryCanceled private @@ -105,12 +114,16 @@ class Namespace .find_by(parent_id: nil) end - def db_nowait_counter - Gitlab::Metrics.counter(:db_nowait, 'Counts the times we triggered NOWAIT on a database lock operation') + def db_query_timeout_counter + Gitlab::Metrics.counter(:db_query_timeout, 'Counts the times the query timed out') end def db_deadlock_counter Gitlab::Metrics.counter(:db_deadlock, 'Counts the times we have deadlocked in the database') end + + def sync_traversal_ids_nowait? + Feature.enabled?(:sync_traversal_ids_nowait, Feature.current_request) + end end end diff --git a/data/deprecations/17-5-ci-job-token-outbound-removal.yml b/data/deprecations/17-5-ci-job-token-outbound-removal.yml new file mode 100644 index 00000000000..a2de2049705 --- /dev/null +++ b/data/deprecations/17-5-ci-job-token-outbound-removal.yml @@ -0,0 +1,38 @@ +# ----- DELETE EVERYTHING ABOVE THIS LINE ----- + +- title: "`ciJobTokenScopeAddProject` GraphQL mutation is deprecated" + # The milestones for the deprecation announcement, and the removal. + removal_milestone: "18.0" + announcement_milestone: "17.5" + # Change breaking_change to false if needed. + breaking_change: true + # The stage and GitLab username of the person reporting the change, + # and a link to the deprecation issue + reporter: jocelynjane + stage: Govern + issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/474175 + impact: low # Can be one of: [critical, high, medium, low] + scope: project # Can be one or a combination of: [instance, group, project] + resolution_role: Developer # Can be one of: [Admin, Owner, Maintainer, Developer] + manual_task: true # Can be true or false. Use this to denote whether a resolution action must be performed manually (true), or if it can be automated by using the API or other automation (false). + body: | # (required) Don't change this line. + With the [upcoming default behavior change to the CI/CD job token](https://docs.gitlab.com/ee/update/deprecations.html#default-cicd-job-token-ci_job_token-scope-changed) in GitLab 18.0, we are also deprecating the associated `ciJobTokenScopeAddProject` GraphQL mutation as the associated feature will be no longer be available. + +# ============================== +# OPTIONAL END-OF-SUPPORT FIELDS +# ============================== +# +# If an End of Support period applies: +# 1) Share this announcement in the `#spt_managers` Support channel in Slack +# 2) Mention `@gitlab-com/support` in this merge request. +# + # When support for this feature ends, in XX.YY milestone format. + end_of_support_milestone: + # Array of tiers the feature is currently available to, + # like [Free, Silver, Gold, Core, Premium, Ultimate] + tiers: + # Links to documentation and thumbnail image + documentation_url: + image_url: + # Use the youtube thumbnail URL with the structure of https://img.youtube.com/vi/UNIQUEID/hqdefault.jpg + video_url: diff --git a/doc/api/protected_branches.md b/doc/api/protected_branches.md index 45f9f604116..2bc6ffe41f6 100644 --- a/doc/api/protected_branches.md +++ b/doc/api/protected_branches.md @@ -396,7 +396,7 @@ Example response: "access_level": null, "user_id": null, "group_id": null, - "deploy_key_id: 1, + "deploy_key_id": 1, "access_level_description": "Deploy" } ], diff --git a/doc/ci/runners/configure_runners.md b/doc/ci/runners/configure_runners.md index 1e47573cd3d..bdcc18fdf46 100644 --- a/doc/ci/runners/configure_runners.md +++ b/doc/ci/runners/configure_runners.md @@ -100,7 +100,7 @@ To set the maximum job timeout: > - [Introduced](https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/4335) in GitLab Runner 16.4. -To control the amount of time `script` and `after_script` runs before it terminates, you can set specify a timeout. +To control the amount of time `script` and `after_script` runs before it terminates, specify a timeout value in the `.gitlab-ci.yml` file. For example, you can specify a timeout to terminate a long-running `script` early, so that artifacts and caches can still be uploaded before the job timeout is exceeded. diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index 55ada4eb9fd..612adace1fe 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -909,6 +909,22 @@ The [`GITLAB_SHARED_RUNNERS_REGISTRATION_TOKEN`](https://docs.gitlab.com/ee/admi
+### `ciJobTokenScopeAddProject` GraphQL mutation is deprecated + +
+ +- Announced in GitLab 17.5 +- Removal in GitLab 18.0 ([breaking change](https://docs.gitlab.com/ee/update/terminology.html#breaking-change)) +- To discuss this change or learn more, see the [deprecation issue](https://gitlab.com/gitlab-org/gitlab/-/issues/474175). + +
+ +With the [upcoming default behavior change to the CI/CD job token](https://docs.gitlab.com/ee/update/deprecations.html#default-cicd-job-token-ci_job_token-scope-changed) in GitLab 18.0, we are also deprecating the associated `ciJobTokenScopeAddProject` GraphQL mutation as the associated feature will be no longer be available. + +
+ +
+ ### `require_password_to_approve` field
diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index 6f13626848a..fe2530c8bd4 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -136,7 +136,7 @@ Audit event types belong to the following product categories. | [`audit_events_streaming_headers_update`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92068) | Triggered when a streaming header for audit events is updated. | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.3](https://gitlab.com/gitlab-org/gitlab/-/issues/366350) | Group | | [`compliance_framework_added`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157893) | Triggered when a compliance framework is applied to a project | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/464160) | Project | | [`compliance_framework_deleted`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/65343) | Triggered when a compliance framework is removed from a project | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [14.1](https://gitlab.com/gitlab-org/gitlab/-/issues/329362) | Project | -| [`compliance_framework_id_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/94711) | Triggered when a compliance framework ID is updated | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/369310) | Project | +| [`compliance_framework_id_updated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/94711) | Triggered when a compliance framework is updated for a project | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.7](https://gitlab.com/gitlab-org/gitlab/-/issues/369310) | Project | | [`compliance_framework_removed`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157893) | Triggered when a compliance framework is removed from a project | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/464160) | Project | | [`create_compliance_framework`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74292) | Triggered on when a compliance framework is successfully created | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/340649) | Group | | [`create_status_check`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/84624) | Triggered when an external status check is created | **{check-circle}** Yes | **{check-circle}** Yes | GitLab [15.9](https://gitlab.com/gitlab-org/gitlab/-/issues/355805) | Project | diff --git a/lib/gitlab/database/transaction/settings.rb b/lib/gitlab/database/transaction/settings.rb new file mode 100644 index 00000000000..9a9086b00e6 --- /dev/null +++ b/lib/gitlab/database/transaction/settings.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Transaction + class Settings + ALLOWED_CONFIGS = %w[ + LOCK_TIMEOUT + ].freeze + + InvalidConfigError = Class.new(StandardError) + + class << self + def with(config_name, value, connection = ApplicationRecord.connection) + old_value = get(config_name, connection) + + set(config_name, value, connection) + + yield + + set(config_name, old_value, connection) + end + + def set(config_name, value, connection = ApplicationRecord.connection) + check_allowed!(config_name) + + quoted_value = connection.quote(value) + query = "SET LOCAL #{config_name} = #{quoted_value}" + + connection.exec_query(query) + end + + def get(config_name, connection = ApplicationRecord.connection) + check_allowed!(config_name) + + connection.select_all("SHOW #{config_name}").rows[0][0] + end + + def check_allowed!(config_name) + return if ALLOWED_CONFIGS.include?(config_name) + + raise InvalidConfigError, "Config #{config_name} is not allowed" + end + end + end + end + end +end diff --git a/lib/gitlab/query_limiting/transaction.rb b/lib/gitlab/query_limiting/transaction.rb index e89559bdb5e..0d599c8c710 100644 --- a/lib/gitlab/query_limiting/transaction.rb +++ b/lib/gitlab/query_limiting/transaction.rb @@ -73,6 +73,8 @@ module Gitlab LICENSES_LOAD = 'SELECT "licenses".* FROM "licenses" ORDER BY "licenses"."id"' SCHEMA_INTROSPECTION = %r{SELECT.*(FROM|JOIN) (pg_attribute|pg_class)}m SAVEPOINT = %r{(RELEASE )?SAVEPOINT}m + SET = %r{^SET\s}m + SHOW = %r{^SHOW\s}m # queries can be safely ignored if they are amoritized in regular usage # (i.e. only requested occasionally and otherwise cached). @@ -81,6 +83,8 @@ module Gitlab return true if sql&.include?(LICENSES_LOAD) return true if SCHEMA_INTROSPECTION.match?(sql) return true if SAVEPOINT.match?(sql) + return true if SET.match?(sql) + return true if SHOW.match?(sql) false end diff --git a/spec/lib/gitlab/database/transaction/settings_spec.rb b/spec/lib/gitlab/database/transaction/settings_spec.rb new file mode 100644 index 00000000000..7b1574b0a21 --- /dev/null +++ b/spec/lib/gitlab/database/transaction/settings_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Transaction::Settings, feature_category: :database do + shared_context 'with a CI transaction' do + before do + skip_if_shared_database(:ci) + end + + around do |example| + Ci::ApplicationRecord.transaction do + example.run + end + end + end + + describe '#get' do + before do + described_class.set('LOCK_TIMEOUT', '200ms') + end + + it 'gets the value of a configuration' do + expect(described_class.get('LOCK_TIMEOUT')).to eq('200ms') + end + + it 'raises an error when config is not allowed' do + expect { described_class.get('some_config') }.to raise_error(described_class::InvalidConfigError) + end + + context 'with connection parameter' do + include_context 'with a CI transaction' + + before do + described_class.set('LOCK_TIMEOUT', '300ms', Ci::Tag.connection) + end + + it 'gets the value of a configuration' do + expect(described_class.get('LOCK_TIMEOUT', Ci::Tag.connection)).to eq('300ms') + end + end + end + + describe '#set' do + it 'sets a configuration value' do + described_class.set('LOCK_TIMEOUT', '300ms') + + expect(described_class.get('LOCK_TIMEOUT')).to eq('300ms') + end + + it 'raises an error when config is not allowed' do + expect { described_class.set('some_config', 10) }.to raise_error(described_class::InvalidConfigError) + end + + context 'with connection parameter' do + include_context 'with a CI transaction' + + it 'sets a configuration value' do + described_class.set('LOCK_TIMEOUT', '300ms') + described_class.set('LOCK_TIMEOUT', '400ms', Ci::Tag.connection) + + expect(described_class.get('LOCK_TIMEOUT')).to eq('300ms') + expect(described_class.get('LOCK_TIMEOUT', Ci::Tag.connection)).to eq('400ms') + end + end + end + + describe '#with' do + it 'sets a configuration value' do + described_class.with('LOCK_TIMEOUT', '400ms') do + expect(described_class.get('LOCK_TIMEOUT')).to eq('400ms') + end + end + + it 'restores original configuration value' do + described_class.set('LOCK_TIMEOUT', '200ms') + + described_class.with('LOCK_TIMEOUT', '500ms') do + expect(described_class.get('LOCK_TIMEOUT')).to eq('500ms') + end + + expect(described_class.get('LOCK_TIMEOUT')).to eq('200ms') + end + + it 'raises an error when config is not allowed' do + expect do + described_class.with('some_config', 10) { 1 + 1 } + end.to raise_error(described_class::InvalidConfigError) + end + + context 'with connection parameter' do + include_context 'with a CI transaction' + + it 'sets and restores a configuration value' do + described_class.set('LOCK_TIMEOUT', '200ms', Ci::Tag.connection) + + described_class.with('LOCK_TIMEOUT', '400ms', Ci::Tag.connection) do + expect(described_class.get('LOCK_TIMEOUT', Ci::Tag.connection)).to eq('400ms') + end + + expect(described_class.get('LOCK_TIMEOUT', Ci::Tag.connection)).to eq('200ms') + end + end + end + + describe '#check_allowed!' do + it 'raises an error when config is not allowed' do + expect { described_class.check_allowed!('some_config') }.to raise_error(described_class::InvalidConfigError) + end + end +end diff --git a/spec/lib/gitlab/query_limiting/transaction_spec.rb b/spec/lib/gitlab/query_limiting/transaction_spec.rb index c9c55a9ece0..7d018a0327c 100644 --- a/spec/lib/gitlab/query_limiting/transaction_spec.rb +++ b/spec/lib/gitlab/query_limiting/transaction_spec.rb @@ -131,6 +131,8 @@ RSpec.describe Gitlab::QueryLimiting::Transaction, feature_category: :database d transaction.increment('SELECT x.foo, a.attname FROM some_table x JOIN pg_attribute a') transaction.increment('SAVEPOINT active_record_2') transaction.increment('RELEASE SAVEPOINT active_record_2') + transaction.increment('SET LOCK_TIMEOUT = 500') + transaction.increment('SHOW LOCK_TIMEOUT') transaction.increment(<<-SQL) SELECT a.attname, a.other_column FROM pg_attribute a diff --git a/spec/models/namespace/traversal_hierarchy_spec.rb b/spec/models/namespace/traversal_hierarchy_spec.rb index 1ffee538ab8..22fbb1a485d 100644 --- a/spec/models/namespace/traversal_hierarchy_spec.rb +++ b/spec/models/namespace/traversal_hierarchy_spec.rb @@ -68,7 +68,7 @@ RSpec.describe Namespace::TraversalHierarchy, type: :model, feature_category: :g end end - it_behaves_like 'locked row', nowait: true do + it_behaves_like 'locked row', timeout: true do let(:recorded_queries) { ActiveRecord::QueryRecorder.new } let(:row) { root } @@ -78,19 +78,20 @@ RSpec.describe Namespace::TraversalHierarchy, type: :model, feature_category: :g end context 'when record is already locked' do + let(:msg) { 'PG::QueryCanceled: ERROR: canceling statement due to statement timeout' } + before do - msg = %(PG::LockNotAvailable: ERROR: could not obtain lock on row in relation "namespaces"\n) - allow(root).to receive(:lock!).and_raise(ActiveRecord::LockWaitTimeout.new(msg)) + allow(root).to receive(:lock!).and_raise(ActiveRecord::QueryCanceled.new(msg)) end - it { expect { subject }.to raise_error(ActiveRecord::LockWaitTimeout) } + it { expect { subject }.to raise_error(ActiveRecord::QueryCanceled, msg) } - it 'increment db_nowait counter' do + it 'increment db_query_timeout counter' do expect do subject rescue StandardError nil - end.to change { db_nowait_total('Namespace#sync_traversal_ids!') }.by(1) + end.to change { db_query_timeout_total('Namespace#sync_traversal_ids!') }.by(1) end end @@ -99,7 +100,7 @@ RSpec.describe Namespace::TraversalHierarchy, type: :model, feature_category: :g stub_feature_flags(sync_traversal_ids_nowait: false) end - it_behaves_like 'locked row', nowait: false do + it_behaves_like 'locked row', timeout: false do let(:recorded_queries) { ActiveRecord::QueryRecorder.new } let(:row) { root } @@ -126,9 +127,9 @@ RSpec.describe Namespace::TraversalHierarchy, type: :model, feature_category: :g end end - def db_nowait_total(source) + def db_query_timeout_total(source) Gitlab::Metrics - .counter(:db_nowait, 'Counts the times we triggered NOWAIT on a database lock operation') + .counter(:db_query_timeout, 'Counts the times the query timed out') .get(source: source) end diff --git a/spec/support/shared_examples/models/label_note_shared_examples.rb b/spec/support/shared_examples/models/label_note_shared_examples.rb index 768d9b82fa9..20ab41a99da 100644 --- a/spec/support/shared_examples/models/label_note_shared_examples.rb +++ b/spec/support/shared_examples/models/label_note_shared_examples.rb @@ -6,19 +6,13 @@ RSpec.shared_examples 'label note created from events' do resource_key = resource.class.name.underscore.to_s event_params[resource_key] = resource - if params[:label].nil? - build(:resource_label_event, event_params.merge(params)).tap do |e| - e.save!(validate: false) - end - else - create(:resource_label_event, event_params.merge(params)) - end + build(:resource_label_event, event_params.merge(params)) end def label_refs(events) labels = events.map(&:label).compact - labels.map { |l| l.to_reference }.sort.join(' ') + labels.map { |l| l.to_reference }.join(' ') end let(:time) { Time.now } @@ -36,8 +30,8 @@ RSpec.shared_examples 'label note created from events' do expect(note.noteable).to eq event.issuable expect(note.note).to be_present expect(note.note_html).to be_present - expect(note.created_at).to be_like_time create_event.created_at - expect(note.updated_at).to be_like_time create_event.created_at + expect(note.created_at).to eq create_event.created_at + expect(note.updated_at).to eq create_event.created_at end it 'updates markdown cache if reference is not set yet' do @@ -74,15 +68,22 @@ RSpec.shared_examples 'label note created from events' do expect(note.note).to eq "added #{label_refs(events)} + 1 deleted label" end - it 'orders label events by id' do + it 'orders label events by label name' do + foo_label = label.dup.tap do |l| + l.update_attribute(:title, 'foo') + end + bar_label = label2.dup.tap do |l| + l.update_attribute(:title, 'bar') + end + events = [ - create_event(created_at: time, label: label), - create_event(created_at: time, label: label2) + create_event(created_at: time, label: foo_label), + create_event(created_at: time, label: bar_label) ] - note = described_class.from_events(events.reverse) + note = described_class.from_events(events) - expect(note.note).to eq "added #{label_refs(events)} labels" + expect(note.note).to eq "added #{label_refs(events.reverse)} labels" end it 'returns text note for removed labels' do diff --git a/spec/support/shared_examples/row_lock_shared_examples.rb b/spec/support/shared_examples/row_lock_shared_examples.rb index 3eed6b42653..ad758e0b3a8 100644 --- a/spec/support/shared_examples/row_lock_shared_examples.rb +++ b/spec/support/shared_examples/row_lock_shared_examples.rb @@ -4,7 +4,7 @@ # Ensure a transaction also occurred. # Be careful! This form of spec is not foolproof, but better than nothing. -RSpec.shared_examples 'locked row' do |nowait: false| +RSpec.shared_examples 'locked row' do |timeout: false| it "has locked row" do table_name = row.class.table_name ids_regex = /SELECT.*FROM.*#{table_name}.*"#{table_name}"."id" = #{row.id}.+FOR NO KEY UPDATE/m @@ -12,11 +12,11 @@ RSpec.shared_examples 'locked row' do |nowait: false| expect(recorded_queries.log).to include a_string_matching 'SAVEPOINT' expect(recorded_queries.log).to include a_string_matching ids_regex - expect(recorded_queries.log).to include a_string_matching 'NOWAIT' if nowait + expect(recorded_queries.log).to include a_string_matching 'LOCK_TIMEOUT' if timeout end end -RSpec.shared_examples 'locked rows' do |nowait: false| +RSpec.shared_examples 'locked rows' do |timeout: false| it "has locked rows" do table_name = rows.first.class.table_name @@ -26,6 +26,6 @@ RSpec.shared_examples 'locked rows' do |nowait: false| expect(recorded_queries.log).to include a_string_matching 'SAVEPOINT' expect(recorded_queries.log).to include a_string_matching ids_regex - expect(recorded_queries.log).to include a_string_matching 'NOWAIT' if nowait + expect(recorded_queries.log).to include a_string_matching 'LOCK_TIMEOUT' if timeout end end