From 35884c636e25123aad202868f39b0c4da6548bf4 Mon Sep 17 00:00:00 2001
From: Gerardo Navarro <gerardo@b310.de>
Date: Wed, 17 Jul 2024 12:18:01 +0000
Subject: [PATCH] GitLab pages: Improve test pages_domain_spec.rb
- Apply table syntax `where` block for `hostname` context
- Remove unnecessary declarations,
e.g. `using RSpec::Parameterized::TableSyntax`
Changelog: other
---
.rubocop_todo/rspec/context_wording.yml | 1 -
.rubocop_todo/rspec/named_subject.yml | 1 -
.rubocop_todo/rspec/return_from_stub.yml | 1 -
spec/models/pages_domain_spec.rb | 160 +++++++++--------------
4 files changed, 61 insertions(+), 102 deletions(-)
diff --git a/.rubocop_todo/rspec/context_wording.yml b/.rubocop_todo/rspec/context_wording.yml
index 3033ce8c5846..e8ec0781bc38 100644
--- a/.rubocop_todo/rspec/context_wording.yml
+++ b/.rubocop_todo/rspec/context_wording.yml
@@ -2078,7 +2078,6 @@ RSpec/ContextWording:
- 'spec/models/packages/dependency_spec.rb'
- 'spec/models/packages/package_file_spec.rb'
- 'spec/models/packages/package_spec.rb'
- - 'spec/models/pages_domain_spec.rb'
- 'spec/models/personal_access_token_spec.rb'
- 'spec/models/plan_limits_spec.rb'
- 'spec/models/preloaders/labels_preloader_spec.rb'
diff --git a/.rubocop_todo/rspec/named_subject.yml b/.rubocop_todo/rspec/named_subject.yml
index 67948df1dc1c..c7876838dc6e 100644
--- a/.rubocop_todo/rspec/named_subject.yml
+++ b/.rubocop_todo/rspec/named_subject.yml
@@ -2650,7 +2650,6 @@ RSpec/NamedSubject:
- 'spec/models/packages/package_file_spec.rb'
- 'spec/models/packages/package_spec.rb'
- 'spec/models/packages/rpm/repository_file_spec.rb'
- - 'spec/models/pages_domain_spec.rb'
- 'spec/models/personal_access_token_spec.rb'
- 'spec/models/plan_limits_spec.rb'
- 'spec/models/project_authorization_spec.rb'
diff --git a/.rubocop_todo/rspec/return_from_stub.yml b/.rubocop_todo/rspec/return_from_stub.yml
index 3d94a601dfc8..343f03e36e40 100644
--- a/.rubocop_todo/rspec/return_from_stub.yml
+++ b/.rubocop_todo/rspec/return_from_stub.yml
@@ -143,7 +143,6 @@ RSpec/ReturnFromStub:
- 'spec/models/internal_id_spec.rb'
- 'spec/models/issue_spec.rb'
- 'spec/models/merge_request_spec.rb'
- - 'spec/models/pages_domain_spec.rb'
- 'spec/models/project_spec.rb'
- 'spec/models/project_statistics_spec.rb'
- 'spec/models/snippet_statistics_spec.rb'
diff --git a/spec/models/pages_domain_spec.rb b/spec/models/pages_domain_spec.rb
index a9d2552d7b77..b51f8a5979f0 100644
--- a/spec/models/pages_domain_spec.rb
+++ b/spec/models/pages_domain_spec.rb
@@ -29,44 +29,41 @@
end
end
- describe 'validate domain' do
+ describe 'domain validations' do
subject(:pages_domain) { build(:pages_domain, domain: domain) }
- context 'is unique' do
+ context 'when the domain is unique' do
let(:domain) { 'my.domain.com' }
it { is_expected.to validate_uniqueness_of(:domain).case_insensitive }
end
- describe "hostname" do
- {
- 'my.domain.com' => true,
- '123.456.789' => true,
- '0x12345.com' => true,
- '0123123' => true,
- 'a-reserved.com' => true,
- 'a.b-reserved.com' => true,
- 'reserved.com' => true,
- '_foo.com' => false,
- 'a.reserved.com' => false,
- 'a.b.reserved.com' => false,
- nil => false
- }.each do |value, validity|
- context "domain #{value.inspect} validity" do
- before do
- allow(Settings.pages).to receive(:host).and_return('reserved.com')
- end
-
- let(:domain) { value }
-
- it { expect(pages_domain.valid?).to eq(validity) }
- end
+ context "with different domain names" do
+ before do
+ allow(Settings.pages).to receive(:host).and_return('reserved.com')
+ end
+
+ where(:domain, :expected) do
+ 'my.domain.com' | true
+ '123.456.789' | true
+ '0x12345.com' | true
+ '0123123' | true
+ 'a-reserved.com' | true
+ 'a.b-reserved.com' | true
+ 'reserved.com' | true
+
+ '_foo.com' | false
+ 'a.reserved.com' | false
+ 'a.b.reserved.com' | false
+ nil | false
+ end
+
+ with_them do
+ it { is_expected.to have_attributes(valid?: expected) }
end
end
describe "HTTPS-only" do
- using RSpec::Parameterized::TableSyntax
-
let(:domain) { 'my.domain.com' }
let(:project) do
@@ -124,7 +121,7 @@
describe 'validate certificate' do
subject { domain }
- context 'serverless domain' do
+ context 'for serverless domain' do
it 'requires certificate and key to be present' do
expect(build(:pages_domain, :without_certificate, :without_key, usage: :serverless)).not_to be_valid
expect(build(:pages_domain, :without_certificate, usage: :serverless)).not_to be_valid
@@ -157,9 +154,7 @@
end
context 'when certificate is expired' do
- let(:domain) do
- build(:pages_domain, :with_trusted_expired_chain)
- end
+ let(:domain) { build(:pages_domain, :with_trusted_expired_chain) }
context 'when certificate is being changed' do
it "adds error to certificate" do
@@ -181,19 +176,17 @@
end
context 'with ecdsa certificate' do
- it "is valid" do
- domain = build(:pages_domain, :ecdsa)
+ let(:domain) { build(:pages_domain, :ecdsa) }
- expect(domain).to be_valid
- end
+ it { is_expected.to be_valid }
context 'when curve is set explicitly by parameters' do
- it 'adds errors to private key' do
- domain = build(:pages_domain, :explicit_ecdsa)
+ let(:domain) { build(:pages_domain, :explicit_ecdsa) }
- expect(domain).to be_invalid
+ it 'adds errors to private key' do
+ is_expected.to be_invalid
- expect(domain.errors[:key]).not_to be_empty
+ expect(domain.errors[:key]).to be_present
end
end
end
@@ -230,20 +223,13 @@
end
describe 'default values' do
- it 'defaults wildcard to false' do
- expect(subject.wildcard).to eq(false)
- end
-
- it 'defaults auto_ssl_enabled to false' do
- expect(subject.auto_ssl_enabled).to eq(false)
- end
-
- it 'defaults scope to project' do
- expect(subject.scope).to eq('project')
- end
-
- it 'defaults usage to pages' do
- expect(subject.usage).to eq('pages')
+ it do
+ is_expected.to have_attributes(
+ wildcard: false,
+ auto_ssl_enabled: false,
+ scope: 'project',
+ usage: 'pages'
+ )
end
end
@@ -251,7 +237,7 @@
subject { pages_domain.verification_code }
it 'is set automatically with 128 bits of SecureRandom data' do
- expect(SecureRandom).to receive(:hex).with(16) { 'verification code' }
+ expect(SecureRandom).to receive(:hex).with(16).and_return('verification code')
is_expected.to eq('verification code')
end
@@ -390,17 +376,13 @@
context 'when certificate is provided by user' do
let(:domain) { create(:pages_domain) }
- it 'returns key' do
- is_expected.to eq(domain.key)
- end
+ it { is_expected.to eq domain.key }
end
context 'when certificate is provided by gitlab' do
let(:domain) { create(:pages_domain, :letsencrypt) }
- it 'returns nil' do
- is_expected.to be_nil
- end
+ it { is_expected.to be_nil }
end
end
@@ -410,24 +392,18 @@
context 'when certificate is provided by user' do
let(:domain) { create(:pages_domain) }
- it 'returns key' do
- is_expected.to eq(domain.certificate)
- end
+ it { is_expected.to eq domain.certificate }
end
context 'when certificate is provided by gitlab' do
let(:domain) { create(:pages_domain, :letsencrypt) }
- it 'returns nil' do
- is_expected.to be_nil
- end
+ it { is_expected.to be_nil }
end
end
shared_examples 'certificate setter' do |attribute, setter_name, old_certificate_source, new_certificate_source|
- let(:domain) do
- create(:pages_domain, certificate_source: old_certificate_source)
- end
+ let(:domain) { create(:pages_domain, certificate_source: old_certificate_source) }
let(:old_value) { domain.public_send(attribute) }
@@ -437,15 +413,13 @@
let(:new_value) { 'new_value' }
it "assignes new value to #{attribute}" do
- expect do
- subject
- end.to change { domain.public_send(attribute) }.from(old_value).to('new_value')
+ expect { subject }
+ .to change { domain.public_send(attribute) }.from(old_value).to('new_value')
end
it 'changes certificate source' do
- expect do
- subject
- end.to change { domain.certificate_source }.from(old_certificate_source).to(new_certificate_source)
+ expect { subject }
+ .to change { domain.certificate_source }.from(old_certificate_source).to(new_certificate_source)
end
end
@@ -489,15 +463,11 @@
let(:domain) { create(:pages_domain, auto_ssl_enabled: true, auto_ssl_failed: true) }
it 'clears failure if auto ssl is disabled' do
- expect do
- domain.update!(auto_ssl_enabled: false)
- end.to change { domain.auto_ssl_failed }.from(true).to(false)
+ expect { domain.update!(auto_ssl_enabled: false) }.to change { domain.auto_ssl_failed }.from(true).to(false)
end
it 'does not clear failure on unrelated updates' do
- expect do
- domain.update!(verified_at: Time.current)
- end.not_to change { domain.auto_ssl_failed }.from(true)
+ expect { domain.update!(verified_at: Time.current) }.not_to change { domain.auto_ssl_failed }.from(true)
end
end
end
@@ -531,24 +501,16 @@
end
describe '.instance_serverless' do
- subject { described_class.instance_serverless }
+ let_it_be(:domain_1) { create(:pages_domain, wildcard: true) }
+ let_it_be(:domain_2) { create(:pages_domain, :instance_serverless) }
+ let_it_be(:domain_3) { create(:pages_domain, scope: :instance) }
+ let_it_be(:domain_4) { create(:pages_domain, :instance_serverless) }
+ let_it_be(:domain_5) { create(:pages_domain, usage: :serverless) }
- before do
- create(:pages_domain, wildcard: true)
- create(:pages_domain, :instance_serverless)
- create(:pages_domain, scope: :instance)
- create(:pages_domain, :instance_serverless)
- create(:pages_domain, usage: :serverless)
- end
+ subject { described_class.instance_serverless }
it 'returns domains that are wildcard, instance-level, and serverless' do
- expect(subject.length).to eq(2)
-
- subject.each do |domain|
- expect(domain.wildcard).to eq(true)
- expect(domain.usage).to eq('serverless')
- expect(domain.scope).to eq('instance')
- end
+ is_expected.to match_array [domain_2, domain_4]
end
end
@@ -610,10 +572,10 @@
end
describe '.find_by_domain_case_insensitive' do
- it 'lookup is case-insensitive' do
- pages_domain = create(:pages_domain, domain: "Pages.IO")
+ let_it_be(:pages_domain) { create(:pages_domain, domain: "Pages.IO") }
- expect(described_class.find_by_domain_case_insensitive('pages.io')).to eq(pages_domain)
+ it 'lookup is case-insensitive' do
+ expect(described_class.find_by_domain_case_insensitive('pages.io')).to eq pages_domain
end
end
end
--
GitLab