Commit 082fa66e authored by Nick Thomas's avatar Nick Thomas

Use rspec-parameterized for table-based tests

parent 0d88bc8e
......@@ -336,6 +336,7 @@ group :development, :test do
gem 'spinach-rerun-reporter', '~> 0.0.2'
gem 'rspec_profiling', '~> 0.0.5'
gem 'rspec-set', '~> 0.1.3'
gem 'rspec-parameterized'
# Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826)
gem 'minitest', '~> 5.7.0'
......
......@@ -2,6 +2,7 @@ GEM
remote: https://rubygems.org/
specs:
RedCloth (4.3.2)
abstract_type (0.0.7)
ace-rails-ap (4.1.2)
actionmailer (4.2.8)
actionpack (= 4.2.8)
......@@ -41,6 +42,9 @@ GEM
tzinfo (~> 1.1)
acts-as-taggable-on (4.0.0)
activerecord (>= 4.0)
adamantium (0.2.0)
ice_nine (~> 0.11.0)
memoizable (~> 0.4.0)
addressable (2.3.8)
after_commit_queue (1.3.0)
activerecord (>= 3.0)
......@@ -132,6 +136,9 @@ GEM
coercible (1.0.0)
descendants_tracker (~> 0.0.1)
colorize (0.7.7)
concord (0.1.5)
adamantium (~> 0.2.0)
equalizer (~> 0.0.9)
concurrent-ruby (1.0.5)
concurrent-ruby-ext (1.0.5)
concurrent-ruby (= 1.0.5)
......@@ -489,6 +496,8 @@ GEM
mime-types (>= 1.16, < 4)
mail_room (0.9.1)
memoist (0.15.0)
memoizable (0.4.2)
thread_safe (~> 0.3, >= 0.3.1)
method_source (0.8.2)
mime-types (2.99.3)
mimemagic (0.3.0)
......@@ -630,6 +639,11 @@ GEM
premailer-rails (1.9.7)
actionmailer (>= 3, < 6)
premailer (~> 1.7, >= 1.7.9)
proc_to_ast (0.1.0)
coderay
parser
unparser
procto (0.0.3)
prometheus-client-mmap (0.7.0.beta11)
mmap2 (~> 2.2, >= 2.2.7)
pry (0.10.4)
......@@ -738,6 +752,10 @@ GEM
chunky_png
rqrcode-rails3 (0.1.7)
rqrcode (>= 0.4.2)
rspec (3.6.0)
rspec-core (~> 3.6.0)
rspec-expectations (~> 3.6.0)
rspec-mocks (~> 3.6.0)
rspec-core (3.6.0)
rspec-support (~> 3.6.0)
rspec-expectations (3.6.0)
......@@ -746,6 +764,12 @@ GEM
rspec-mocks (3.6.0)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.6.0)
rspec-parameterized (0.4.0)
binding_of_caller
parser
proc_to_ast
rspec (>= 2.13, < 4)
unparser
rspec-rails (3.6.0)
actionpack (>= 3.0)
activesupport (>= 3.0)
......@@ -913,6 +937,14 @@ GEM
get_process_mem (~> 0)
unicorn (>= 4, < 6)
uniform_notifier (1.10.0)
unparser (0.2.6)
abstract_type (~> 0.0.7)
adamantium (~> 0.2.0)
concord (~> 0.1.5)
diff-lcs (~> 1.3)
equalizer (~> 0.0.9)
parser (>= 2.3.1.2, < 2.5)
procto (~> 0.0.2)
url_safe_base64 (0.2.2)
validates_hostname (1.0.6)
activerecord (>= 3.0)
......@@ -1123,6 +1155,7 @@ DEPENDENCIES
responders (~> 2.0)
rouge (~> 2.0)
rqrcode-rails3 (~> 0.1.7)
rspec-parameterized
rspec-rails (~> 3.6.0)
rspec-retry (~> 0.4.5)
rspec-set (~> 0.1.3)
......
......@@ -268,6 +268,43 @@ end
- Avoid scenario titles that add no information, such as "successfully".
- Avoid scenario titles that repeat the feature title.
### Table-based / Parameterized tests
This style of testing is used to exercise one piece of code with a comprehensive
range of inputs. By specifying the test case once, alongside a table of inputs
and the expected output for each, your tests can be made easier to read and more
compact.
We use the [rspec-parameterized](https://github.com/tomykaira/rspec-parameterized)
gem. A short example, using the table syntax and checking Ruby equality for a
range of inputs, might look like this:
```ruby
describe "#==" do
using Rspec::Parameterized::TableSyntax
let(:project1) { create(:project) }
let(:project2) { create(:project) }
where(:a, :b, :result) do
1 | 1 | true
1 | 2 | false
true | true | true
true | false | false
project1 | project1 | true
project2 | project2 | true
project 1 | project2 | false
end
with_them do
it { expect(a == b).to eq(result) }
it 'is isomorphic' do
expect(b == a).to eq(result)
end
end
end
```
### Matchers
Custom matchers should be created to clarify the intent and/or hide the
......
require 'spec_helper'
describe Issue do
using RSpec::Parameterized::TableSyntax
describe '#allows_multiple_assignees?' do
it 'does not allow multiple assignees without license' do
stub_licensed_features(multiple_issue_assignees: false)
......@@ -20,24 +22,23 @@ describe Issue do
end
describe '#weight' do
[
{ license: true, database: 5, expected: 5 },
{ license: true, database: nil, expected: nil },
{ license: false, database: 5, expected: nil },
{ license: false, database: nil, expected: nil }
].each do |spec|
context spec.inspect do
let(:spec) { spec }
let(:issue) { build(:issue, weight: spec[:database]) }
subject { issue.weight }
before do
stub_licensed_features(issue_weights: spec[:license])
end
it { is_expected.to eq(spec[:expected]) }
where(:license_value, :database_value, :expected) do
true | 5 | 5
true | nil | nil
false | 5 | nil
false | nil | nil
end
with_them do
let(:issue) { build(:issue, weight: database_value) }
subject { issue.weight }
before do
stub_licensed_features(issue_weights: license_value)
end
it { is_expected.to eq(expected) }
end
end
end
require 'spec_helper'
describe MergeRequest do
using RSpec::Parameterized::TableSyntax
let(:project) { create(:project, :repository) }
subject(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
......@@ -135,24 +137,23 @@ describe MergeRequest do
end
describe '#approvals_before_merge' do
[
{ license: true, database: 5, expected: 5 },
{ license: true, database: nil, expected: nil },
{ license: false, database: 5, expected: nil },
{ license: false, database: nil, expected: nil }
].each do |spec|
context spec.inspect do
let(:spec) { spec }
let(:merge_request) { build(:merge_request, approvals_before_merge: spec[:database]) }
subject { merge_request.approvals_before_merge }
before do
stub_licensed_features(merge_request_approvers: spec[:license])
end
it { is_expected.to eq(spec[:expected]) }
where(:license_value, :db_value, :expected) do
true | 5 | 5
true | nil | nil
false | 5 | nil
false | nil | nil
end
with_them do
let(:merge_request) { build(:merge_request, approvals_before_merge: db_value) }
subject { merge_request.approvals_before_merge }
before do
stub_licensed_features(merge_request_approvers: license_value)
end
it { is_expected.to eq(expected) }
end
end
end
require 'spec_helper'
describe ProjectImportData do
using RSpec::Parameterized::TableSyntax
let(:import_url) { 'ssh://example.com' }
let(:import_data_attrs) { { auth_method: 'ssh_public_key' } }
let(:project) { build(:project, :mirror, import_url: import_url, import_data_attributes: import_data_attrs) }
......@@ -12,20 +14,19 @@ describe ProjectImportData do
end
describe '#ssh_key_auth?' do
subject { import_data.ssh_key_auth? }
[
{ import_url: 'ssh://example.com', auth_method: 'ssh_public_key', expected: true },
{ import_url: 'ssh://example.com', auth_method: 'password', expected: false },
{ import_url: 'http://example.com', auth_method: 'ssh_public_key', expected: false },
{ import_url: 'http://example.com', auth_method: 'password', expected: false }
].each do |spec|
context spec.inspect do
let(:import_url) { spec[:import_url] }
let(:import_data_attrs) { { auth_method: spec[:auth_method] } }
it { is_expected.to spec[:expected] ? be_truthy : be_falsy }
end
where(:import_url, :auth_method, :expected) do
'ssh://example.com' | 'ssh_public_key' | true
'ssh://example.com' | 'password' | false
'http://example.com' | 'ssh_public_key' | false
'http://example.com' | 'password' | false
end
with_them do
let(:import_data_attrs) { { auth_method: auth_method } }
subject { import_data.ssh_key_auth? }
it { is_expected.to eq(expected) }
end
end
......@@ -62,20 +63,18 @@ describe ProjectImportData do
end
describe '#ssh_import?' do
subject { import_data.ssh_import? }
[
{ import_url: nil, expected: false },
{ import_url: 'ssh://example.com', expected: true },
{ import_url: 'git://example.com', expected: false },
{ import_url: 'http://example.com', expected: false },
{ import_url: 'https://example.com', expected: false }
].each do |spec|
context spec.inspect do
let(:import_url) { spec[:import_url] }
it { is_expected.to spec[:expected] ? be_truthy : be_falsy }
end
where(:import_url, :expected) do
'ssh://example.com' | true
'git://example.com' | false
'http://example.com' | false
'https://example.com' | false
nil | nil
end
with_them do
subject { import_data.ssh_import? }
it { is_expected.to eq(expected) }
end
end
......
require 'spec_helper'
describe Project do
using RSpec::Parameterized::TableSyntax
describe 'associations' do
it { is_expected.to delegate_method(:shared_runners_minutes).to(:statistics) }
it { is_expected.to delegate_method(:shared_runners_seconds).to(:statistics) }
......@@ -639,102 +641,98 @@ describe Project do
end
describe '#approvals_before_merge' do
[
{ license: true, database: 5, expected: 5 },
{ license: true, database: 0, expected: 0 },
{ license: false, database: 5, expected: 0 },
{ license: false, database: 0, expected: 0 }
].each do |spec|
context spec.inspect do
let(:spec) { spec }
let(:project) { build(:project, approvals_before_merge: spec[:database]) }
subject { project.approvals_before_merge }
where(:license_value, :db_value, :expected) do
true | 5 | 5
true | 0 | 0
false | 5 | 0
false | 0 | 0
end
before do
stub_licensed_features(merge_request_approvers: spec[:license])
end
with_them do
let(:project) { build(:project, approvals_before_merge: db_value) }
subject { project.approvals_before_merge }
it { is_expected.to eq(spec[:expected]) }
before do
stub_licensed_features(merge_request_approvers: license_value)
end
it { is_expected.to eq(expected) }
end
end
describe "#reset_approvals_on_push?" do
[
{ license: true, database: true, expected: true },
{ license: true, database: false, expected: false },
{ license: false, database: true, expected: false },
{ license: false, database: false, expected: false }
].each do |spec|
context spec.inspect do
let(:spec) { spec }
let(:project) { build(:project, reset_approvals_on_push: spec[:database]) }
subject { project.reset_approvals_on_push? }
where(:license_value, :db_value, :expected) do
true | true | true
true | false | false
false | true | false
false | false | false
end
before do
stub_licensed_features(merge_request_approvers: spec[:license])
end
with_them do
let(:project) { build(:project, reset_approvals_on_push: db_value) }
subject { project.reset_approvals_on_push? }
it { is_expected.to eq(spec[:expected]) }
before do
stub_licensed_features(merge_request_approvers: license_value)
end
it { is_expected.to eq(expected) }
end
end
describe '#approvals_before_merge' do
[
{ license: true, database: 5, expected: 5 },
{ license: true, database: 0, expected: 0 },
{ license: false, database: 5, expected: 0 },
{ license: false, database: 0, expected: 0 }
].each do |spec|
context spec.inspect do
let(:spec) { spec }
let(:project) { build(:project, approvals_before_merge: spec[:database]) }
subject { project.approvals_before_merge }
where(:license_value, :db_value, :expected) do
true | 5 | 5
true | 0 | 0
false | 5 | 0
false | 0 | 0
end
before do
stub_licensed_features(merge_request_approvers: spec[:license])
end
with_them do
let(:project) { build(:project, approvals_before_merge: db_value) }
subject { project.approvals_before_merge }
it { is_expected.to eq(spec[:expected]) }
before do
stub_licensed_features(merge_request_approvers: license_value)
end
it { is_expected.to eq(expected) }
end
end
describe '#merge_method' do
[
{ ff: true, rebase: true, ff_licensed: true, rebase_licensed: true, method: :ff },
{ ff: true, rebase: true, ff_licensed: true, rebase_licensed: false, method: :ff },
{ ff: true, rebase: true, ff_licensed: false, rebase_licensed: true, method: :rebase_merge },
{ ff: true, rebase: true, ff_licensed: false, rebase_licensed: false, method: :merge },
{ ff: true, rebase: false, ff_licensed: true, rebase_licensed: true, method: :ff },
{ ff: true, rebase: false, ff_licensed: true, rebase_licensed: false, method: :ff },
{ ff: true, rebase: false, ff_licensed: false, rebase_licensed: true, method: :merge },
{ ff: true, rebase: false, ff_licensed: false, rebase_licensed: false, method: :merge },
{ ff: false, rebase: true, ff_licensed: true, rebase_licensed: true, method: :rebase_merge },
{ ff: false, rebase: true, ff_licensed: true, rebase_licensed: false, method: :merge },
{ ff: false, rebase: true, ff_licensed: false, rebase_licensed: true, method: :rebase_merge },
{ ff: false, rebase: true, ff_licensed: false, rebase_licensed: false, method: :merge },
{ ff: false, rebase: false, ff_licensed: true, rebase_licensed: true, method: :merge },
{ ff: false, rebase: false, ff_licensed: true, rebase_licensed: false, method: :merge },
{ ff: false, rebase: false, ff_licensed: false, rebase_licensed: true, method: :merge },
{ ff: false, rebase: false, ff_licensed: false, rebase_licensed: false, method: :merge }
].each do |spec|
context spec.inspect do
let(:project) { build(:project, merge_requests_rebase_enabled: spec[:rebase], merge_requests_ff_only_enabled: spec[:ff]) }
let(:spec) { spec }
subject { project.merge_method }
before do
stub_licensed_features(merge_request_rebase: spec[:rebase_licensed], fast_forward_merge: spec[:ff_licensed])
end
where(:ff, :rebase, :ff_licensed, :rebase_licensed, :method) do
true | true | true | true | :ff
true | true | true | false | :ff
true | true | false | true | :rebase_merge
true | true | false | false | :merge
true | false | true | true | :ff
true | false | true | false | :ff
true | false | false | true | :merge
true | false | false | false | :merge
false | true | true | true | :rebase_merge
false | true | true | false | :merge
false | true | false | true | :rebase_merge
false | true | false | false | :merge
false | false | true | true | :merge
false | false | true | false | :merge
false | false | false | true | :merge
false | false | false | false | :merge
end
with_them do
let(:project) { build(:project, merge_requests_rebase_enabled: rebase, merge_requests_ff_only_enabled: ff) }
subject { project.merge_method }
it { is_expected.to eq(spec[:method]) }
before do
stub_licensed_features(merge_request_rebase: rebase_licensed, fast_forward_merge: ff_licensed)
end
it { is_expected.to eq(method) }
end
end
......
require 'spec_helper'
describe SshHostKey do
using RSpec::Parameterized::TableSyntax
include ReactiveCachingHelpers
keys = [
SSHKeygen.generate,
SSHKeygen.generate
]
let(:key1) do
'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC3UpyF2iLqy1d63M6k3jH1vuEnq/NWtE+o' \
'rJe1Xn7JoRbduKd6zpsJ0JhBGWgcQK0ph0aGW5PcudzzBSc+SlYfCc4GTaxDtmj41hW0o72m' \
'NiuDW3oKXXShOiVRde2ZOquH8Z865jGiZIC8BI/bXZD29IGUih0hPu7Rjp70VYiE+35QRf/p' \
'sD0Ddrz8QUIG3A/2dMzLI5F5ZORk3BIX2F3mJwJOvZxRhR/SqyphDMZ5eZ0EzqbFBCDE6HAB' \
'Woz9ck8RBGLvCIggmDHj3FmMLcQGMDiy6wKp7QdnBtxjCP6vtE6YPUM223AqsWt+9NTtCfB8' \
'YdNAH7YcHHOR1FgtSk1x'
end
let(:key2) do
'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDLIp+4ciR2YO9f9rpldc7InNQw/TBUtcNb' \
'J2XR0rr15/5ytz7YM16xXG0Qjx576PNSmqs4gbTrvTuFZak+v1Jx/9deHRq/yqp9f+tv33+i' \
'aJGCQCX/+OVY7aWgV2R9YsS7XQ4mnv4XlOTEssib/rGAIT+ATd/GcdYSEOO+dh4O09/6O/jI' \
'MGSeP+NNetgn1nPCnLOjrXFZUnUtNDi6EEKeIlrliJjSb7Jr4f7gjvZnv4RskWHHFo8FgAAq' \
't0gOMT6EmKrnypBe2vLGSAXbtkXr01q6/DNPH+n9VA1LTV6v1KN/W5CN5tQV11wRSKiM8g5O' \
'Ebi86VjJRi2sOuYoXQU1'
end
# Purposefully ordered so that `sort` will make changes
known_hosts = <<~EOF
example.com #{keys[0]} git@localhost
@revoked other.example.com #{keys[1]} git@localhost
EOF
let(:known_hosts) do
<<~EOF
example.com #{key1} git@localhost
@revoked other.example.com #{key2} git@localhost
EOF
end
let(:extra) { known_hosts + "foo\nbar\n" }
let(:reversed) { known_hosts.lines.reverse.join }
def stub_ssh_keyscan(args, status: true, stdout: "", stderr: "")
stdin = StringIO.new
......@@ -33,7 +52,7 @@ describe SshHostKey do
it 'returns an array of indexed fingerprints when the cache is filled' do
stub_reactive_cache(ssh_host_key, known_hosts: known_hosts)
expected = keys
expected = [key1, key2]
.map { |data| Gitlab::KeyFingerprint.new(data) }
.each_with_index
.map { |key, i| { bits: key.bits, fingerprint: key.fingerprint, type: key.type, index: i } }
......@@ -48,16 +67,12 @@ describe SshHostKey do
describe '#fingerprints', use_clean_rails_memory_store_caching: true do
it 'returns an array of indexed fingerprints when the cache is filled' do
key1 = SSHKeygen.generate
key2 = SSHKeygen.generate
known_hosts = "example.com #{key1} git@localhost\n\n\n@revoked other.example.com #{key2} git@localhost\n"
stub_reactive_cache(ssh_host_key, known_hosts: known_hosts)
expect(ssh_host_key.fingerprints.as_json).to eq(
[
{ bits: 2048, fingerprint: Gitlab::KeyFingerprint.new(key1).fingerprint, type: 'RSA', index: 0 },
{ bits: 2048, fingerprint: Gitlab::KeyFingerprint.new(key2).fingerprint, type: 'RSA', index: 3 }
{ bits: 2048, fingerprint: Gitlab::KeyFingerprint.new(key2).fingerprint, type: 'RSA', index: 1 }
]
)
end
......@@ -68,36 +83,35 @@ describe SshHostKey do
end
describe '#changes_project_import_data?' do
subject { ssh_host_key.changes_project_import_data? }
reversed = known_hosts.lines.reverse.join
extra = known_hosts + "foo\nbar\n"
[
{ a: known_hosts, b: extra, result: true },
{ a: known_hosts, b: "foo\n", result: true },
{ a: known_hosts, b: '', result: true },
{ a: known_hosts, b: nil, result: true },
{ a: known_hosts, b: known_hosts, result: false },
{ a: reversed, b: known_hosts, result: false },
{ a: extra, b: "foo\n", result: true },
{ a: '', b: '', result: false },
{ a: nil, b: nil, result: false },
{ a: '', b: nil, result: false }
].each_with_index do |spec, index|
it "is #{spec[:result]} for test case #{index}" do
expect(ssh_host_key).to receive(:known_hosts).and_return(spec[:a])
project.import_data.ssh_known_hosts = spec[:b]
is_expected.to eq(spec[:result])
where(:a, :b, :result) do
known_hosts | extra | true
known_hosts | "foo\n" | true
known_hosts | '' | true
known_hosts | nil | true
known_hosts | known_hosts | false
reversed | known_hosts | false
extra | "foo\n" | true
'' | '' | false
nil | nil | false
'' | nil | false
end
with_them do
subject { ssh_host_key.changes_project_import_data? }
it "(normal)" do
expect(ssh_host_key).to receive(:known_hosts).and_return(a)
project.import_data.ssh_known_hosts = b
is_expected.to eq(result)
end
# Comparisons should be symmetrical, so test the reverse too
it "is #{spec[:result]} for test case #{index} (reversed)" do
expect(ssh_host_key).to receive(:known_hosts).and_return(spec[:b])
project.import_data.ssh_known_hosts = spec[:a]
it "(reversed)" do
expect(ssh_host_key).to receive(:known_hosts).and_return(b)
project.import_data.ssh_known_hosts = a
is_expected.to eq(spec[:result])
is_expected.to eq(result)
end
end
end
......
......@@ -8,6 +8,7 @@ require File.expand_path("../../config/environment", __FILE__)
require 'rspec/rails'
require 'shoulda/matchers'
require 'rspec/retry'
require 'rspec-parameterized'
rspec_profiling_is_configured =
ENV['RSPEC_PROFILING_POSTGRES_URL'].present? ||
......
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