Commit c306eda8 authored by Pedro Pombeiro's avatar Pedro Pombeiro

Address MR review comments

parent 1d2c83cf
......@@ -13,9 +13,7 @@ module Gitlab
end
def valid?
strong_memoize(:valid) do
errors.nil?
end
errors.nil?
end
# errors sorts an array of variables, ignoring unknown variable references,
......@@ -26,7 +24,7 @@ module Gitlab
strong_memoize(:errors) do
# Check for cyclic dependencies and build error message in that case
errors = each_strongly_connected_component.filter_map do |component|
"#{component.map { |v| v[:key] }.inspect}" if component.size > 1
component.map { |v| v[:key] }.inspect if component.size > 1
end
"circular variable reference detected: #{errors.join(', ')}" if errors.any?
......@@ -38,16 +36,14 @@ module Gitlab
def sort
return @variables if Feature.disabled?(:variable_inside_variable)
clear_memoization(:valid)
clear_memoization(:errors)
begin
# Perform a topological sort
variables = tsort
valid = true
strong_memoize(:errors) { nil }
rescue TSort::Cyclic
variables = @variables
valid = false
end
strong_memoize(:valid) { valid }
variables
end
......
......@@ -3,15 +3,11 @@
require 'spec_helper'
RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
let(:variable_inside_variable_enabled) { false }
before do
stub_feature_flags(variable_inside_variable: variable_inside_variable_enabled)
end
describe '#errors' do
context 'when FF :variable_inside_variable is disabled' do
let(:variable_inside_variable_enabled) { false }
before do
stub_feature_flags(variable_inside_variable: false)
end
context 'table tests' do
using RSpec::Parameterized::TableSyntax
......@@ -71,7 +67,9 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
end
context 'when FF :variable_inside_variable is enabled' do
let(:variable_inside_variable_enabled) { true }
before do
stub_feature_flags(variable_inside_variable: true)
end
context 'table tests' do
using RSpec::Parameterized::TableSyntax
......@@ -118,7 +116,9 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
describe '#sort' do
context 'when FF :variable_inside_variable is disabled' do
let(:variable_inside_variable_enabled) { false }
before do
stub_feature_flags(variable_inside_variable: false)
end
context 'table tests' do
using RSpec::Parameterized::TableSyntax
......@@ -174,7 +174,11 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
end
context 'when FF :variable_inside_variable is enabled' do
let(:variable_inside_variable_enabled) { true }
before do
stub_licensed_features(group_saml_group_sync: true)
stub_feature_flags(saml_group_links: true)
stub_feature_flags(variable_inside_variable: true)
end
context 'table tests' do
using RSpec::Parameterized::TableSyntax
......
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