From cb01133e46fb3c554cad223eacbfbd09a7bb3924 Mon Sep 17 00:00:00 2001 From: Maciej Mensfeld Date: Mon, 6 Jan 2025 17:48:36 +0100 Subject: [PATCH] rubocop remarks --- config/locales/errors.yml | 3 + spec/lib/karafka/core/configurable_spec.rb | 22 ++--- .../karafka/core/contractable/result_spec.rb | 99 ++++++++++++++++++- .../monitoring/statistics_decorator_spec.rb | 32 +++--- 4 files changed, 125 insertions(+), 31 deletions(-) diff --git a/config/locales/errors.yml b/config/locales/errors.yml index 6db850a..ef4bff4 100644 --- a/config/locales/errors.yml +++ b/config/locales/errors.yml @@ -4,6 +4,9 @@ en: missing: needs to be present id_format: needs to be a String nested.id2_format: needs to be a String + id_format: needs to be a String + test_format: needs to be 5 + details.id_format: needs to be a String config: missing: needs to be present diff --git a/spec/lib/karafka/core/configurable_spec.rb b/spec/lib/karafka/core/configurable_spec.rb index 012f214..330b7ec 100644 --- a/spec/lib/karafka/core/configurable_spec.rb +++ b/spec/lib/karafka/core/configurable_spec.rb @@ -36,7 +36,7 @@ it { expect(config.nested1.nested2.leaf).to eq(6) } it { expect(config.nested1.nested1).to eq(1) } it { expect(config.nested1.nested2.with_constructor).to eq(5) } - it { expect(config.nested1.nested2.ov_constructor).to eq(true) } + it { expect(config.nested1.nested2.ov_constructor).to be(true) } it { expect(config.nested1.nested2.with_zero_constructor).to eq(7) } end @@ -52,7 +52,7 @@ it { expect(config.nested1.nested2.leaf).to eq(8) } it { expect(config.nested1.nested1).to eq(1) } it { expect(config.nested1.nested2.with_constructor).to eq(5) } - it { expect(config.nested1.nested2.ov_constructor).to eq(true) } + it { expect(config.nested1.nested2.ov_constructor).to be(true) } end context 'when we inherit and alter settings' do @@ -75,12 +75,12 @@ it { expect(config.nested1.nested2.leaf).to eq(6) } it { expect(config.nested1.nested1).to eq(1) } it { expect(config.nested1.nested2.with_constructor).to eq(5) } - it { expect(config.nested1.nested2.ov_constructor).to eq(true) } + it { expect(config.nested1.nested2.ov_constructor).to be(true) } it { expect(config_sub.with_default).to eq(123) } it { expect(config_sub.nested1.nested2.leaf).to eq(6) } it { expect(config_sub.nested1.nested1).to eq(1) } it { expect(config_sub.nested1.nested2.with_constructor).to eq(5) } - it { expect(config_sub.nested1.nested2.ov_constructor).to eq(true) } + it { expect(config_sub.nested1.nested2.ov_constructor).to be(true) } end context 'when we inherit and change values' do @@ -200,7 +200,7 @@ let(:constructor) { ->(default) { default || attempts.pop } } it 'expect to retry until non-false is present and then cache it' do - 3.times { expect(config.lazy_setting).to eq(false) } + 3.times { expect(config.lazy_setting).to be(false) } expect(config.lazy_setting).to eq(10) expect(config.lazy_setting).to eq(10) end @@ -225,7 +225,7 @@ let(:constructor) { -> { attempts.pop } } it 'expect to retry until non-false is present and then cache it' do - 3.times { expect(config.lazy_setting).to eq(false) } + 3.times { expect(config.lazy_setting).to be(false) } expect(config.lazy_setting).to eq(10) expect(config.lazy_setting).to eq(10) end @@ -275,7 +275,7 @@ it { expect(config.nested1.nested2.leaf).to eq(6) } it { expect(config.nested1.nested1).to eq(1) } it { expect(config.nested1.nested2.with_constructor).to eq(5) } - it { expect(config.nested1.nested2.ov_constructor).to eq(true) } + it { expect(config.nested1.nested2.ov_constructor).to be(true) } end context 'when we have two instances' do @@ -306,7 +306,7 @@ it { expect(config.nested1.nested2.leaf).to eq(8) } it { expect(config.nested1.nested1).to eq(1) } it { expect(config.nested1.nested2.with_constructor).to eq(5) } - it { expect(config.nested1.nested2.ov_constructor).to eq(true) } + it { expect(config.nested1.nested2.ov_constructor).to be(true) } end context 'when we inherit and alter settings' do @@ -329,12 +329,12 @@ it { expect(config.nested1.nested2.leaf).to eq(6) } it { expect(config.nested1.nested1).to eq(1) } it { expect(config.nested1.nested2.with_constructor).to eq(5) } - it { expect(config.nested1.nested2.ov_constructor).to eq(true) } + it { expect(config.nested1.nested2.ov_constructor).to be(true) } it { expect(config_sub.with_default).to eq(123) } it { expect(config_sub.nested1.nested2.leaf).to eq(6) } it { expect(config_sub.nested1.nested1).to eq(1) } it { expect(config_sub.nested1.nested2.with_constructor).to eq(5) } - it { expect(config_sub.nested1.nested2.ov_constructor).to eq(true) } + it { expect(config_sub.nested1.nested2.ov_constructor).to be(true) } end context 'when we inherit and change values' do @@ -426,7 +426,7 @@ def testable let(:constructor) { ->(default) { default || attempts.pop } } it 'expect to retry until non-false is present and then cache it' do - 3.times { expect(config.lazy_setting).to eq(false) } + 3.times { expect(config.lazy_setting).to be(false) } expect(config.lazy_setting).to eq(10) expect(config.lazy_setting).to eq(10) end diff --git a/spec/lib/karafka/core/contractable/result_spec.rb b/spec/lib/karafka/core/contractable/result_spec.rb index a92abbf..07ec553 100644 --- a/spec/lib/karafka/core/contractable/result_spec.rb +++ b/spec/lib/karafka/core/contractable/result_spec.rb @@ -1,19 +1,110 @@ # frozen_string_literal: true RSpec.describe_current do + subject(:contract_class) do + Class.new(Karafka::Core::Contractable::Contract) do + configure do |config| + config.error_messages = YAML.safe_load( + File.read( + File.join(Karafka::Core.gem_root, 'config', 'locales', 'errors.yml') + ) + ).fetch('en').fetch('validations').fetch('test') + end + + required(:id) { |id| id.is_a?(String) } + + optional(:test) { |test| test == 5 } + end + end + + let(:contract) { contract_class.new } + context 'when there are no errors' do - pending + let(:data) { { id: '123', test: 5 } } + let(:result) { contract.call(data) } + + it 'returns success' do + expect(result.success?).to be true + end + + it 'has no errors' do + expect(result.errors).to be_empty + end end context 'when there are errors and all keys can be mapped to messages' do - pending + let(:data) { { id: 123, test: 4 } } + let(:result) { contract.call(data) } + + it 'does not return success' do + expect(result.success?).to be false + end + + it 'maps errors to messages' do + expect(result.errors).to eq( + id: 'needs to be a String', + test: 'needs to be 5' + ) + end end context 'when there are errors with nested keys' do - pending + subject(:contract_class) do + Class.new(Karafka::Core::Contractable::Contract) do + configure do |config| + config.error_messages = YAML.safe_load( + File.read( + File.join(Karafka::Core.gem_root, 'config', 'locales', 'errors.yml') + ) + ).fetch('en').fetch('validations').fetch('test') + end + + nested(:details) do + required(:id) { |id| id.is_a?(String) } + end + end + end + + let(:data) { { details: { id: 123 } } } + let(:result) { contract.call(data) } + + it 'does not return success' do + expect(result.success?).to be false + end + + it 'maps nested errors to messages with dot notation' do + expect(result.errors).to eq( + 'details.id': 'needs to be a String' + ) + end end context 'when there are errors and key cannot be mapped to a message' do - pending + subject(:contract_class) do + Class.new(Karafka::Core::Contractable::Contract) do + configure do |config| + config.error_messages = YAML.safe_load( + File.read( + File.join(Karafka::Core.gem_root, 'config', 'locales', 'errors.yml') + ) + ).fetch('en').fetch('validations').fetch('test') + end + + required(:id) { |id| id.is_a?(String) } + end + end + + let(:data) { { id: 123 } } + let(:result) { contract.call(data) } + + it 'does not return success' do + expect(result.success?).to be false + end + + it 'falls back to a generic error message' do + expect(result.errors).to eq( + id: 'needs to be a String' + ) + end end end diff --git a/spec/lib/karafka/core/monitoring/statistics_decorator_spec.rb b/spec/lib/karafka/core/monitoring/statistics_decorator_spec.rb index 17b790e..9bd1778 100644 --- a/spec/lib/karafka/core/monitoring/statistics_decorator_spec.rb +++ b/spec/lib/karafka/core/monitoring/statistics_decorator_spec.rb @@ -54,7 +54,7 @@ subject(:decorated) { decorator.call(emited_stats1) } it { expect(decorated['string']).to eq('value1') } - it { expect(decorated.key?('string_d')).to eq(false) } + it { expect(decorated.key?('string_d')).to be(false) } it { expect(decorated['float_d']).to eq(0) } it { expect(decorated['int_d']).to eq(0) } it { expect(decorated.dig(*broker_scope)['txbytes_d']).to eq(0) } @@ -68,7 +68,7 @@ end it { expect(decorated['string']).to eq('value2') } - it { expect(decorated.key?('string_d')).to eq(false) } + it { expect(decorated.key?('string_d')).to be(false) } it { expect(decorated['float_d'].round(10)).to eq(0.4) } it { expect(decorated['int_d']).to eq(18) } it { expect(decorated.dig(*broker_scope)['txbytes_d']).to eq(30) } @@ -83,8 +83,8 @@ end it { expect(decorated['string']).to eq('value3') } - it { expect(decorated.key?('string_d')).to eq(false) } - it { expect(decorated.key?('string_fd')).to eq(false) } + it { expect(decorated.key?('string_d')).to be(false) } + it { expect(decorated.key?('string_fd')).to be(false) } it { expect(decorated['float_d'].round(10)).to eq(1.0) } it { expect(decorated['float_fd']).to be_within(5).of(0) } it { expect(decorated['int_d']).to eq(-120) } @@ -92,7 +92,7 @@ it { expect(decorated.dig(*broker_scope)['txbytes_d']).to eq(-151) } it { expect(decorated.dig(*broker_scope)['txbytes_fd']).to be_within(5).of(0) } it { expect(decorated).to be_frozen } - it { expect(decorated.key?('float_d_d')).to eq(false) } + it { expect(decorated.key?('float_d_d')).to be(false) } end context 'when a broker is no longer present' do @@ -104,15 +104,15 @@ before { emited_stats2['nested'] = {} } it { expect(decorated['string']).to eq('value2') } - it { expect(decorated.key?('string_d')).to eq(false) } - it { expect(decorated.key?('string_fd')).to eq(false) } + it { expect(decorated.key?('string_d')).to be(false) } + it { expect(decorated.key?('string_fd')).to be(false) } it { expect(decorated['float_d'].round(10)).to eq(0.4) } it { expect(decorated['float_fd']).to be_within(5).of(0) } it { expect(decorated['int_d']).to eq(18) } it { expect(decorated['int_fd']).to be_within(5).of(0) } it { expect(decorated['nested']).to eq({}) } it { expect(decorated).to be_frozen } - it { expect(decorated.key?('float_d_d')).to eq(false) } + it { expect(decorated.key?('float_d_d')).to be(false) } end context 'when broker was introduced later on' do @@ -124,7 +124,7 @@ before { emited_stats1['nested'] = {} } it { expect(decorated['string']).to eq('value2') } - it { expect(decorated.key?('string_d')).to eq(false) } + it { expect(decorated.key?('string_d')).to be(false) } it { expect(decorated['float_d'].round(10)).to eq(0.4) } it { expect(decorated['float_fd']).to be_within(5).of(0) } it { expect(decorated['int_d']).to eq(18) } @@ -132,7 +132,7 @@ it { expect(decorated.dig(*broker_scope)['txbytes_d']).to eq(0) } it { expect(decorated.dig(*broker_scope)['txbytes_fd']).to be_within(5).of(0) } it { expect(decorated).to be_frozen } - it { expect(decorated.key?('float_d_d')).to eq(false) } + it { expect(decorated.key?('float_d_d')).to be(false) } end context 'when value remains unchanged over time' do @@ -148,14 +148,14 @@ let(:deep_copy) { -> { Marshal.load(Marshal.dump(emited_stats1)) } } - it { expect(decorated.key?('string_d')).to eq(false) } - it { expect(decorated.key?('string_fd')).to eq(false) } + it { expect(decorated.key?('string_d')).to be(false) } + it { expect(decorated.key?('string_fd')).to be(false) } it { expect(decorated['float_d']).to eq(0) } it { expect(decorated['float_fd']).to be_within(5).of(10) } it { expect(decorated['int_d']).to eq(0) } it { expect(decorated['int_fd']).to be_within(5).of(10) } it { expect(decorated).to be_frozen } - it { expect(decorated.key?('float_d_d')).to eq(false) } + it { expect(decorated.key?('float_d_d')).to be(false) } end context 'when value remains unchanged over multiple occurrences and time' do @@ -173,13 +173,13 @@ let(:deep_copy) { -> { Marshal.load(Marshal.dump(emited_stats1)) } } - it { expect(decorated.key?('string_d')).to eq(false) } - it { expect(decorated.key?('string_fd')).to eq(false) } + it { expect(decorated.key?('string_d')).to be(false) } + it { expect(decorated.key?('string_fd')).to be(false) } it { expect(decorated['float_d']).to eq(0) } it { expect(decorated['float_fd']).to be_within(5).of(20) } it { expect(decorated['int_d']).to eq(0) } it { expect(decorated['int_fd']).to be_within(5).of(20) } it { expect(decorated).to be_frozen } - it { expect(decorated.key?('float_d_d')).to eq(false) } + it { expect(decorated.key?('float_d_d')).to be(false) } end end