From 3ed80a0176a0c8155ff6f84a8f3e70718babd8ce Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Mon, 19 Sep 2016 19:28:41 +0100 Subject: Enforce the fork_project permission in Projects::CreateService Projects::ForkService delegates to this service almost entirely, but needed one small change so it would propagate create errors correctly. CreateService#execute needs significant refactoring; it is now right at the complexity limit set by Rubocop. I avoided doing so in this commit to keep the diff as small as possible. Several tests depend on the insecure behaviour of ForkService, so fi them up at the same time. --- spec/models/forked_project_link_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/models') diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index 9c81d159cdf..1863581f57b 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -6,6 +6,7 @@ describe ForkedProjectLink, "add link on fork" do let(:user) { create(:user, namespace: namespace) } before do + create(:project_member, :reporter, user: user, project: project_from) @project_to = fork_project(project_from, user) end -- cgit v1.2.1 From 8dfec32d2bac661313f62b338b6d991b3fa25b98 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 3 Oct 2016 18:22:51 +0800 Subject: Rename ignored to failed_but_allowed, introduce exclude_ignored which merges previous exclude_ignored_jobs and failed_but_allowed, so that we don't treat ignored a special case in HasStatus. --- spec/models/build_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index e7864b7ad33..ae185de9ca3 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 -- cgit v1.2.1 From 752a4cce514f04e0f364f615b817e64e9f40879b Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 3 Oct 2016 18:56:32 +0800 Subject: Add test for CommitStatus.exclude_ignored --- spec/models/commit_status_spec.rb | 55 ++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 15 deletions(-) (limited to 'spec/models') diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 2f1baff5d66..80c2a1bc7a9 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 -- cgit v1.2.1 From afc0ae5cbef721dad867c2fca9734fa9748b02f2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 3 Oct 2016 19:35:53 +0800 Subject: Fix tests. Check 'success' first (default status) --- spec/models/concerns/has_status_spec.rb | 43 +++++++++++++++------------------ 1 file changed, 20 insertions(+), 23 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index e118432d098..87bffbdc54e 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 -- cgit v1.2.1 From 45bfadbcbf0eec5bf7c691b13751f684e3c62c5c Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Fri, 30 Sep 2016 11:53:44 -0300 Subject: Fix bug when trying to cache closed issues from external issue trackers --- spec/models/merge_request_spec.rb | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'spec/models') diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 9d7be2429ed..556b4c1efc3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -86,6 +86,30 @@ describe MergeRequest, models: true do end end + describe '#cache_merge_request_closes_issues!' do + before do + subject.project.team << [subject.author, :developer] + subject.target_branch = subject.project.default_branch + end + + it 'caches closed issues' do + issue = create :issue, project: subject.project + commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") + allow(subject).to receive(:commits).and_return([commit]) + + expect { subject.cache_merge_request_closes_issues! }.to change(subject.merge_requests_closing_issues, :count).by(1) + end + + it 'does not cache issues from external trackers' do + subject.project.update_attribute(:has_external_issue_tracker, true) + issue = ExternalIssue.new('JIRA-123', subject.project) + commit = double('commit1', safe_message: "Fixes #{issue.to_reference}") + allow(subject).to receive(:commits).and_return([commit]) + + expect { subject.cache_merge_request_closes_issues! }.not_to change(subject.merge_requests_closing_issues, :count) + end + end + describe '#source_branch_sha' do let(:last_branch_commit) { subject.source_project.repository.commit(subject.source_branch) } -- cgit v1.2.1 From 901c994b7a4481437f8fe91583d2ed3f19e4775e Mon Sep 17 00:00:00 2001 From: "Z.J. van de Weg" Date: Mon, 3 Oct 2016 15:39:12 +0200 Subject: deployment refs in own folder, new method for creating refs --- spec/models/repository_spec.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'spec/models') diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index db29f4d353b..98c64c079b9 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -320,6 +320,16 @@ describe Repository, models: true do end end + describe '#create_ref' do + it 'redirects the call to fetch_ref' do + ref, ref_path = '1', '2' + + expect(repository).to receive(:fetch_ref).with(repository.path_to_repo, ref, ref_path) + + repository.create_ref(ref, ref_path) + end + end + describe "#changelog" do before do repository.send(:cache).expire(:changelog) -- cgit v1.2.1 From 912d7f7b68d5c75d801299a8c9363a257030e142 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Tue, 4 Oct 2016 10:01:32 +0200 Subject: Fix issues importing services via Import/Export --- spec/models/service_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'spec/models') diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 05056a4bb47..ed1bc9271ae 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -203,6 +203,23 @@ describe Service, models: true do end end + describe 'initialize service with no properties' do + let(:service) do + GitlabIssueTrackerService.create( + project: create(:project), + title: 'random title' + ) + end + + it 'does not raise error' do + expect { service }.not_to raise_error + end + + it 'creates the properties' do + expect(service.properties).to eq({ "title" => "random title" }) + end + end + describe "callbacks" do let(:project) { create(:project) } let!(:service) do -- cgit v1.2.1 From 4ff345c488c993e9f67f34ad271bc98a1f3ffa61 Mon Sep 17 00:00:00 2001 From: Paco Guzman Date: Thu, 29 Sep 2016 16:28:45 +0200 Subject: Simplify Mentionable concern instance methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We remove some arguments that are rarely used or used just to simplify setups on specs. Modified Mentionable#create_new_cross_references method we don’t need to calculate previous references to avoid the duplication because we do that at database level when creating references extracted from the current entity state. MergeRequests won’t create cross_references for commits that are included so we change a spec to use a different merge request to make references to commits to other branches --- spec/models/concerns/mentionable_spec.rb | 22 +++++++++++++++------- spec/models/merge_request_spec.rb | 2 +- 2 files changed, 16 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 549b0042038..132858950d5 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -1,18 +1,27 @@ require 'spec_helper' describe Mentionable do - include Mentionable + class Example + include Mentionable - def author - nil + attr_accessor :project, :message + attr_mentionable :message + + def author + nil + end end describe 'references' do let(:project) { create(:project) } + let(:mentionable) { Example.new } it 'excludes JIRA references' do allow(project).to receive_messages(jira_tracker?: true) - expect(referenced_mentionables(project, 'JIRA-123')).to be_empty + + mentionable.project = project + mentionable.message = 'JIRA-123' + expect(mentionable.referenced_mentionables).to be_empty end end end @@ -39,9 +48,8 @@ describe Issue, "Mentionable" do let(:user) { create(:user) } def referenced_issues(current_user) - text = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}" - - issue.referenced_mentionables(current_user, text) + issue.title = "#{private_issue.to_reference(project)} and #{public_issue.to_reference}" + issue.referenced_mentionables(current_user) end context 'when the current user can see the issue' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 9d7be2429ed..c29364466f0 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -522,7 +522,7 @@ describe MergeRequest, models: true do end it_behaves_like 'an editable mentionable' do - subject { create(:merge_request) } + subject { create(:merge_request, :simple) } let(:backref_text) { "merge request #{subject.to_reference}" } let(:set_mentionable_text) { ->(txt){ subject.description = txt } } -- cgit v1.2.1 From 5ce5abdc1dc53e97cff88f5abfe0edd10211bf9c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 4 Oct 2016 12:30:57 +0200 Subject: Fix hipchat service specs after changes in pipeline --- spec/models/project_services/hipchat_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index cf713684463..26dd95bdfec 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -283,7 +283,7 @@ describe HipchatService, models: true do context 'build events' do let(:pipeline) { create(:ci_empty_pipeline) } let(:build) { create(:ci_build, pipeline: pipeline) } - let(:data) { Gitlab::DataBuilder::Build.build(build) } + let(:data) { Gitlab::DataBuilder::Build.build(build.reload) } context 'for failed' do before { build.drop } -- cgit v1.2.1 From 706737a004b67303f15ccbd1f8630b0d80f481e9 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 4 Oct 2016 13:21:26 +0200 Subject: Exclude system notes from Project.trending Having many system notes isn't really an indication of a project being trending. Including these notes would lead to projects with lots of commit cross references (for example) showing up in the trending projects list. --- spec/models/project_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ef854a25321..1a316176f63 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -826,6 +826,14 @@ describe Project, models: true do expect(subject).to eq([project2, project1]) end end + + it 'does not take system notes into account' do + 10.times do + create(:note_on_commit, project: project2, system: true) + end + + expect(described_class.trending.to_a).to eq([project1, project2]) + end end describe '.visible_to_user' do -- cgit v1.2.1 From c9bcfc631a79db97203d51d7c221a7549ba65adf Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 4 Oct 2016 17:22:58 +0200 Subject: Remove lease from Event#reset_project_activity Per GitLab.com's performance metrics this method could take up to 5 seconds of wall time to complete, while only taking 1-2 milliseconds of CPU time. Removing the Redis lease in favour of conditional updates allows us to work around this. A slight drawback is that this allows for multiple threads/processes to try and update the same row. However, only a single thread/process will ever win since the UPDATE query uses a WHERE condition to only update rows that were not updated in the last hour. Fixes gitlab-org/gitlab-ce#22473 --- spec/models/event_spec.rb | 8 +++----- spec/models/project_spec.rb | 4 +--- 2 files changed, 4 insertions(+), 8 deletions(-) (limited to 'spec/models') diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb index 8600eb4d2c4..af5002487cc 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 ef854a25321..3ab5ac78bba 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) -- cgit v1.2.1 From 16ed9b6129daf51a296d4576580c5f232d043db6 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 4 Oct 2016 18:03:10 +0200 Subject: Refactor Gitlab::Identifier This refactors Gitlab::Identifier so it uses fewer queries and is actually tested. Queries are reduced by caching the output as well as using 1 query (instead of 2) to find a user using an SSH key. --- spec/models/user_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'spec/models') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a1770d96f83..65b2896930a 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) } -- cgit v1.2.1 From e94cd6fdfe43d9128d37a539cf84f4388c5cf970 Mon Sep 17 00:00:00 2001 From: Nick Thomas Date: Thu, 6 Oct 2016 22:17:11 +0100 Subject: Add markdown cache columns to the database, but don't use them yet This commit adds a number of _html columns and, with the exception of Note, starts updating them whenever the content of their partner fields changes. Note has a collision with the note_html attr_accessor; that will be fixed later A background worker for clearing these cache columns is also introduced - use `rake cache:clear` to set it off. You can clear the database or Redis caches separately by running `rake cache:clear:db` or `rake cache:clear:redis`, respectively. --- spec/models/abuse_report_spec.rb | 4 + spec/models/concerns/cache_markdown_field_spec.rb | 181 ++++++++++++++++++++++ spec/models/project_spec.rb | 2 +- spec/models/service_spec.rb | 2 +- spec/models/snippet_spec.rb | 7 + 5 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 spec/models/concerns/cache_markdown_field_spec.rb (limited to 'spec/models') diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 305f8bc88cc..c4486a32082 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -9,6 +9,10 @@ RSpec.describe AbuseReport, type: :model do describe 'associations' do it { is_expected.to belong_to(:reporter).class_name('User') } it { is_expected.to belong_to(:user) } + + it "aliases reporter to author" do + expect(subject.author).to be(subject.reporter) + end end describe 'validations' do diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb new file mode 100644 index 00000000000..15cd3a7ed70 --- /dev/null +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -0,0 +1,181 @@ +require 'spec_helper' + +describe CacheMarkdownField do + CacheMarkdownField::CACHING_CLASSES << "ThingWithMarkdownFields" + + # The minimum necessary ActiveModel to test this concern + class ThingWithMarkdownFields + include ActiveModel::Model + include ActiveModel::Dirty + + include ActiveModel::Serialization + + class_attribute :attribute_names + self.attribute_names = [] + + def attributes + attribute_names.each_with_object({}) do |name, hsh| + hsh[name.to_s] = send(name) + end + end + + extend ActiveModel::Callbacks + define_model_callbacks :save + + include CacheMarkdownField + cache_markdown_field :foo + cache_markdown_field :baz, pipeline: :single_line + + def self.add_attr(attr_name) + self.attribute_names += [attr_name] + define_attribute_methods(attr_name) + attr_reader(attr_name) + define_method("#{attr_name}=") do |val| + send("#{attr_name}_will_change!") unless val == send(attr_name) + instance_variable_set("@#{attr_name}", val) + end + end + + [:foo, :foo_html, :bar, :baz, :baz_html].each do |attr_name| + add_attr(attr_name) + end + + def initialize(*) + super + + # Pretend new is load + clear_changes_information + end + + def save + run_callbacks :save do + changes_applied + end + end + end + + CacheMarkdownField::CACHING_CLASSES.delete("ThingWithMarkdownFields") + + def thing_subclass(new_attr) + Class.new(ThingWithMarkdownFields) { add_attr(new_attr) } + end + + let(:markdown) { "`Foo`" } + let(:html) { "

Foo

" } + + let(:updated_markdown) { "`Bar`" } + let(:updated_html) { "

Bar

" } + + subject { ThingWithMarkdownFields.new(foo: markdown, foo_html: html) } + + describe ".attributes" do + it "excludes cache attributes" do + expect(thing_subclass(:qux).new.attributes.keys.sort).to eq(%w[bar baz foo qux]) + end + end + + describe ".cache_markdown_field" do + it "refuses to allow untracked classes" do + expect { thing_subclass(:qux).__send__(:cache_markdown_field, :qux) }.to raise_error(RuntimeError) + end + end + + context "an unchanged markdown field" do + before do + subject.foo = subject.foo + subject.save + end + + it { expect(subject.foo).to eq(markdown) } + it { expect(subject.foo_html).to eq(html) } + it { expect(subject.foo_html_changed?).not_to be_truthy } + end + + context "a changed markdown field" do + before do + subject.foo = updated_markdown + subject.save + end + + it { expect(subject.foo_html).to eq(updated_html) } + end + + context "a non-markdown field changed" do + before do + subject.bar = "OK" + subject.save + end + + it { expect(subject.bar).to eq("OK") } + it { expect(subject.foo).to eq(markdown) } + it { expect(subject.foo_html).to eq(html) } + end + + describe '#banzai_render_context' do + it "sets project to nil if the object lacks a project" do + context = subject.banzai_render_context(:foo) + expect(context).to have_key(:project) + expect(context[:project]).to be_nil + end + + it "excludes author if the object lacks an author" do + context = subject.banzai_render_context(:foo) + expect(context).not_to have_key(:author) + end + + it "raises if the context for an unrecognised field is requested" do + expect{subject.banzai_render_context(:not_found)}.to raise_error(ArgumentError) + end + + it "includes the pipeline" do + context = subject.banzai_render_context(:baz) + expect(context[:pipeline]).to eq(:single_line) + end + + it "returns copies of the context template" do + template = subject.cached_markdown_fields[:baz] + copy = subject.banzai_render_context(:baz) + expect(copy).not_to be(template) + end + + context "with a project" do + subject { thing_subclass(:project).new(foo: markdown, foo_html: html, project: :project) } + + it "sets the project in the context" do + context = subject.banzai_render_context(:foo) + expect(context).to have_key(:project) + expect(context[:project]).to eq(:project) + end + + it "invalidates the cache when project changes" do + subject.project = :new_project + allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html) + + subject.save + + expect(subject.foo_html).to eq(updated_html) + expect(subject.baz_html).to eq(updated_html) + end + end + + context "with an author" do + subject { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author) } + + it "sets the author in the context" do + context = subject.banzai_render_context(:foo) + expect(context).to have_key(:author) + expect(context[:author]).to eq(:author) + end + + it "invalidates the cache when author changes" do + subject.author = :new_author + allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html) + + subject.save + + expect(subject.foo_html).to eq(updated_html) + expect(subject.baz_html).to eq(updated_html) + end + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e52d4aaf884..381d14ed21a 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -518,7 +518,7 @@ describe Project, models: true do end describe '#cache_has_external_issue_tracker' do - let(:project) { create(:project) } + let(:project) { create(:project, has_external_issue_tracker: nil) } it 'stores true if there is any external_issue_tracker' do services = double(:service, external_issue_trackers: [RedmineService.new]) diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index ed1bc9271ae..43937a54b2c 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -238,7 +238,7 @@ describe Service, models: true do it "updates the has_external_issue_tracker boolean" do expect do service.save! - end.to change { service.project.has_external_issue_tracker }.from(nil).to(true) + end.to change { service.project.has_external_issue_tracker }.from(false).to(true) end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index e6bc5296398..f62f6bacbaa 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -46,6 +46,13 @@ describe Snippet, models: true do end end + describe "#content_html_invalidated?" do + let(:snippet) { create(:snippet, content: "md", content_html: "html", file_name: "foo.md") } + it "invalidates the HTML cache of content when the filename changes" do + expect { snippet.file_name = "foo.rb" }.to change { snippet.content_html_invalidated? }.from(false).to(true) + end + end + describe '.search' do let(:snippet) { create(:snippet) } -- cgit v1.2.1 From 1662640985b56390a4d22dab1fee7fd04ccd5bc8 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 6 Oct 2016 15:50:29 -0700 Subject: Fix Event#reset_project_activity updates !6678 removed the lease from Event#reset_project_activity, but it wasn't actually updating the project's last_activity_at timestamp properly. The WHERE clause would always return no matching projects. The spec passed occasionally because the created_at timestamp was automatically set to last_activity_at. --- spec/models/project_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e52d4aaf884..67a968aaf11 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -308,7 +308,9 @@ describe Project, models: true do end describe 'last_activity methods' do - let(:project) { create(:project, last_activity_at: 2.hours.ago) } + let(:timestamp) { 2.hours.ago } + # last_activity_at gets set to created_at upon creation + let(:project) { create(:project, created_at: timestamp, updated_at: timestamp) } describe 'last_activity' do it 'alias last_activity to last_event' do @@ -322,6 +324,7 @@ describe Project, models: true do it 'returns the creation date of the project\'s last event if present' do new_event = create(:event, project: project, created_at: Time.now) + project.reload expect(project.last_activity_at.to_i).to eq(new_event.created_at.to_i) end -- cgit v1.2.1