Commit 3b934ec0 authored by Patrick Steinhardt's avatar Patrick Steinhardt

checks: Fix mismatch in `#new_commits()` signature

In commit 0a90028f (checks: Speed up retrieving commits via
quarantine directory, 2021-07-29), the Gitlab::Git::Repository class got
a new optional parameter `allow_quarnatine` for `#new_commits()`. The
commit forgot to amend the Repository model though to accept the same
parameter, which as a result leads to a runtime error when the access
checks try to load new commits.

Fix this issue by adding the missing parameter to the Repository model.
This commit also changes the changes access spec to call the original
function such that the bug would've been caught without the fix.

Changelog: fixed
parent cf8ebf70
......@@ -168,8 +168,8 @@ class Repository
end
# Returns a list of commits that are not present in any reference
def new_commits(newrev)
commits = raw.new_commits(newrev)
def new_commits(newrev, allow_quarantine: false)
commits = raw.new_commits(newrev, allow_quarantine: allow_quarantine)
::Commit.decorate(commits, container)
end
......
......@@ -60,7 +60,7 @@ RSpec.describe Gitlab::Checks::ChangesAccess do
describe '#commits' do
it 'calls #new_commits' do
expect(project.repository).to receive(:new_commits).and_return([])
expect(project.repository).to receive(:new_commits).and_call_original
expect(subject.commits).to eq([])
end
......
......@@ -398,32 +398,47 @@ RSpec.describe Repository do
end
describe '#new_commits' do
let_it_be(:project) { create(:project, :repository) }
shared_examples '#new_commits' do
let_it_be(:project) { create(:project, :repository) }
let(:repository) { project.repository }
let(:repository) { project.repository }
subject { repository.new_commits(rev) }
subject { repository.new_commits(rev, allow_quarantine: allow_quarantine) }
context 'when there are no new commits' do
let(:rev) { repository.commit.id }
context 'when there are no new commits' do
let(:rev) { repository.commit.id }
it 'returns an empty array' do
expect(subject).to eq([])
it 'returns an empty array' do
expect(subject).to eq([])
end
end
end
context 'when new commits are found' do
let(:branch) { 'orphaned-branch' }
let!(:rev) { repository.commit(branch).id }
context 'when new commits are found' do
let(:branch) { 'orphaned-branch' }
let!(:rev) { repository.commit(branch).id }
let(:allow_quarantine) { false }
it 'returns the commits' do
repository.delete_branch(branch)
it 'returns the commits' do
repository.delete_branch(branch)
expect(subject).not_to be_empty
expect(subject).to all( be_a(::Commit) )
expect(subject.size).to eq(1)
expect(subject).not_to be_empty
expect(subject).to all( be_a(::Commit) )
expect(subject.size).to eq(1)
end
end
end
context 'with quarantine' do
let(:allow_quarantine) { true }
it_behaves_like '#new_commits'
end
context 'without quarantine' do
let(:allow_quarantine) { false }
it_behaves_like '#new_commits'
end
end
describe '#commits_by' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment