Commit d64f1312 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu

Merge branch 'remove-ci-if-parent' into 'master'

Remove `ci_if_parenthesis_enabled` feature flag

Closes #238174

See merge request gitlab-org/gitlab!42583
parents 3810ab0f 51b03f1e
---
name: ci_if_parenthesis_enabled
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37574
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/238174
group: group::ci
type: development
default_enabled: true
\ No newline at end of file
...@@ -39,10 +39,6 @@ module Gitlab ...@@ -39,10 +39,6 @@ module Gitlab
::Feature.enabled?(:ci_bulk_insert_on_create, project, default_enabled: true) ::Feature.enabled?(:ci_bulk_insert_on_create, project, default_enabled: true)
end end
def self.ci_if_parenthesis_enabled?
::Feature.enabled?(:ci_if_parenthesis_enabled, default_enabled: true)
end
# NOTE: The feature flag `disallow_to_create_merge_request_pipelines_in_target_project` # NOTE: The feature flag `disallow_to_create_merge_request_pipelines_in_target_project`
# is a safe switch to disable the feature for a parituclar project when something went wrong, # is a safe switch to disable the feature for a parituclar project when something went wrong,
# therefore it's not supposed to be enabled by default. # therefore it's not supposed to be enabled by default.
......
...@@ -24,26 +24,8 @@ module Gitlab ...@@ -24,26 +24,8 @@ module Gitlab
Expression::Lexeme::Or Expression::Lexeme::Or
].freeze ].freeze
# To be removed with `ci_if_parenthesis_enabled`
LEGACY_LEXEMES = [
Expression::Lexeme::Variable,
Expression::Lexeme::String,
Expression::Lexeme::Pattern,
Expression::Lexeme::Null,
Expression::Lexeme::Equals,
Expression::Lexeme::Matches,
Expression::Lexeme::NotEquals,
Expression::Lexeme::NotMatches,
Expression::Lexeme::And,
Expression::Lexeme::Or
].freeze
def self.lexemes def self.lexemes
if ::Gitlab::Ci::Features.ci_if_parenthesis_enabled? LEXEMES
LEXEMES
else
LEGACY_LEXEMES
end
end end
MAX_TOKENS = 100 MAX_TOKENS = 100
......
...@@ -15,12 +15,7 @@ module Gitlab ...@@ -15,12 +15,7 @@ module Gitlab
def tree def tree
results = [] results = []
tokens = tokens = tokens_rpn
if ::Gitlab::Ci::Features.ci_if_parenthesis_enabled?
tokens_rpn
else
legacy_tokens_rpn
end
tokens.each do |token| tokens.each do |token|
case token.type case token.type
...@@ -78,27 +73,6 @@ module Gitlab ...@@ -78,27 +73,6 @@ module Gitlab
output.concat(operators.reverse) output.concat(operators.reverse)
end end
# To be removed with `ci_if_parenthesis_enabled`
def legacy_tokens_rpn
output = []
operators = []
@tokens.each do |token|
case token.type
when :value
output.push(token)
when :logical_operator
if operators.any? && token.lexeme.precedence >= operators.last.lexeme.precedence
output.push(operators.pop)
end
operators.push(token)
end
end
output.concat(operators.reverse)
end
end end
end end
end end
......
...@@ -90,24 +90,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexer do ...@@ -90,24 +90,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Lexer do
end end
with_them do with_them do
context 'when ci_if_parenthesis_enabled is enabled' do it { is_expected.to eq(tokens) }
before do
stub_feature_flags(ci_if_parenthesis_enabled: true)
end
it { is_expected.to eq(tokens) }
end
context 'when ci_if_parenthesis_enabled is disabled' do
before do
stub_feature_flags(ci_if_parenthesis_enabled: false)
end
it do
expect { subject }
.to raise_error described_class::SyntaxError
end
end
end end
end end
end end
......
...@@ -3,10 +3,6 @@ ...@@ -3,10 +3,6 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Ci::Pipeline::Expression::Parser do RSpec.describe Gitlab::Ci::Pipeline::Expression::Parser do
before do
stub_feature_flags(ci_if_parenthesis_enabled: true)
end
describe '#tree' do describe '#tree' do
context 'validates simple operators' do context 'validates simple operators' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
...@@ -31,36 +27,15 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Parser do ...@@ -31,36 +27,15 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Parser do
context 'when combining && and OR operators' do context 'when combining && and OR operators' do
subject { described_class.seed('$VAR1 == "a" || $VAR2 == "b" && $VAR3 == "c" || $VAR4 == "d" && $VAR5 == "e"').tree } subject { described_class.seed('$VAR1 == "a" || $VAR2 == "b" && $VAR3 == "c" || $VAR4 == "d" && $VAR5 == "e"').tree }
context 'when parenthesis engine is enabled' do it 'returns operations in a correct order' do
before do expect(subject.inspect)
stub_feature_flags(ci_if_parenthesis_enabled: true) .to eq('or(or(equals($VAR1, "a"), and(equals($VAR2, "b"), equals($VAR3, "c"))), and(equals($VAR4, "d"), equals($VAR5, "e")))')
end
it 'returns operations in a correct order' do
expect(subject.inspect)
.to eq('or(or(equals($VAR1, "a"), and(equals($VAR2, "b"), equals($VAR3, "c"))), and(equals($VAR4, "d"), equals($VAR5, "e")))')
end
end
context 'when parenthesis engine is disabled (legacy)' do
before do
stub_feature_flags(ci_if_parenthesis_enabled: false)
end
it 'returns operations in a invalid order' do
expect(subject.inspect)
.to eq('or(equals($VAR1, "a"), and(equals($VAR2, "b"), or(equals($VAR3, "c"), and(equals($VAR4, "d"), equals($VAR5, "e")))))')
end
end end
end end
context 'when using parenthesis' do context 'when using parenthesis' do
subject { described_class.seed('(($VAR1 == "a" || $VAR2 == "b") && $VAR3 == "c" || $VAR4 == "d") && $VAR5 == "e"').tree } subject { described_class.seed('(($VAR1 == "a" || $VAR2 == "b") && $VAR3 == "c" || $VAR4 == "d") && $VAR5 == "e"').tree }
before do
stub_feature_flags(ci_if_parenthesis_enabled: true)
end
it 'returns operations in a correct order' do it 'returns operations in a correct order' do
expect(subject.inspect) expect(subject.inspect)
.to eq('and(or(and(or(equals($VAR1, "a"), equals($VAR2, "b")), equals($VAR3, "c")), equals($VAR4, "d")), equals($VAR5, "e"))') .to eq('and(or(and(or(equals($VAR1, "a"), equals($VAR2, "b")), equals($VAR3, "c")), equals($VAR4, "d")), equals($VAR5, "e"))')
...@@ -96,38 +71,21 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Parser do ...@@ -96,38 +71,21 @@ RSpec.describe Gitlab::Ci::Pipeline::Expression::Parser do
end end
context 'when parenthesis are unmatched' do context 'when parenthesis are unmatched' do
context 'when parenthesis engine is enabled' do where(:expression) do
before do [
stub_feature_flags(ci_if_parenthesis_enabled: true) '$VAR == (',
end '$VAR2 == ("aa"',
'$VAR2 == ("aa"))',
where(:expression) do '$VAR2 == "aa")',
[ '(($VAR2 == "aa")',
'$VAR == (', '($VAR2 == "aa"))'
'$VAR2 == ("aa"', ]
'$VAR2 == ("aa"))',
'$VAR2 == "aa")',
'(($VAR2 == "aa")',
'($VAR2 == "aa"))'
]
end
with_them do
it 'raises a ParseError' do
expect { described_class.seed(expression).tree }
.to raise_error Gitlab::Ci::Pipeline::Expression::Parser::ParseError
end
end
end end
context 'when parenthesis engine is disabled' do with_them do
before do it 'raises a ParseError' do
stub_feature_flags(ci_if_parenthesis_enabled: false) expect { described_class.seed(expression).tree }
end .to raise_error Gitlab::Ci::Pipeline::Expression::Parser::ParseError
it 'raises an SyntaxError' do
expect { described_class.seed('$VAR == (').tree }
.to raise_error Gitlab::Ci::Pipeline::Expression::Lexer::SyntaxError
end end
end end
end end
......
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