From 640f9ee8e188b16d50c7c83795e3289572aa37b1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 7 Mar 2018 12:31:41 +0100 Subject: [PATCH 1/4] Improve tests for predefined variables for a build --- app/models/ci/build.rb | 2 +- spec/models/ci/build_spec.rb | 124 +++++++++++++++++++++++++---------- 2 files changed, 89 insertions(+), 37 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b230b7f47ef..79e473c85b7 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -268,7 +268,7 @@ module Ci variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule variables += persisted_environment_variables if environment - variables + variables.reverse.uniq { |variable| variable.fetch(:key) }.reverse end def features diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c27313ed88b..692c4cc81e0 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1424,6 +1424,17 @@ describe Ci::Build do { key: 'CI_COMMIT_SHA', value: build.sha, public: true }, { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true }, { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true }, + { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true }, + { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false }, + { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false }, + { key: 'CI_BUILD_ID', value: build.id.to_s, public: true }, + { key: 'CI_BUILD_TOKEN', value: build.token, public: false }, + { key: 'CI_BUILD_REF', value: build.sha, public: true }, + { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true }, + { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true }, + { key: 'CI_BUILD_REF_SLUG', value: build.ref_slug, public: true }, + { key: 'CI_BUILD_NAME', value: 'test', public: true }, + { key: 'CI_BUILD_STAGE', value: 'test', public: true }, { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true }, { key: 'CI_PROJECT_NAME', value: project.path, public: true }, { key: 'CI_PROJECT_PATH', value: project.full_path, public: true }, @@ -1433,9 +1444,7 @@ describe Ci::Build do { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true }, { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true }, { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true }, - { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true }, - { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false }, - { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false } + { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true } ] end @@ -1834,39 +1843,6 @@ describe Ci::Build do it { is_expected.to include(ci_config_path) } end - context 'returns variables in valid order' do - let(:build_pre_var) { { key: 'build', value: 'value' } } - let(:project_pre_var) { { key: 'project', value: 'value' } } - let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } } - let(:build_yaml_var) { { key: 'yaml', value: 'value' } } - - before do - allow(build).to receive(:predefined_variables) { [build_pre_var] } - allow(build).to receive(:yaml_variables) { [build_yaml_var] } - - allow_any_instance_of(Project) - .to receive(:predefined_variables) { [project_pre_var] } - - allow_any_instance_of(Project) - .to receive(:secret_variables_for) - .with(ref: 'master', environment: nil) do - [create(:ci_variable, key: 'secret', value: 'value')] - end - - allow_any_instance_of(Ci::Pipeline) - .to receive(:predefined_variables) { [pipeline_pre_var] } - end - - it do - is_expected.to eq( - [build_pre_var, - project_pre_var, - pipeline_pre_var, - build_yaml_var, - { key: 'secret', value: 'value', public: false }]) - end - end - context 'when using auto devops' do context 'and is enabled' do before do @@ -1890,6 +1866,82 @@ describe Ci::Build do end end end + + context 'when pipeline variable overrides build variable' do + before do + build.yaml_variables = [{ key: 'MYVAR', value: 'myvar', public: true }] + pipeline.variables.build(key: 'MYVAR', value: 'pipeline value') + end + + it 'removes duplicates and leaves the latest occurence' do + expect(subject.count { |variable| variable.fetch(:key) == 'MYVAR' }) + .to be 1 + end + + it 'overrides YAML variable using a pipeline variable' do + is_expected.not_to include(key: 'MYVAR', value: 'myvar', public: true) + is_expected.to include(key: 'MYVAR', value: 'pipeline value', public: false) + end + end + + describe 'variables ordering' do + context 'when variables hierarchy is stubbed' do + let(:build_pre_var) { { key: 'build', value: 'value' } } + let(:project_pre_var) { { key: 'project', value: 'value' } } + let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } } + let(:build_yaml_var) { { key: 'yaml', value: 'value' } } + + before do + allow(build).to receive(:predefined_variables) { [build_pre_var] } + allow(build).to receive(:yaml_variables) { [build_yaml_var] } + + allow_any_instance_of(Project) + .to receive(:predefined_variables) { [project_pre_var] } + + allow_any_instance_of(Project) + .to receive(:secret_variables_for) + .with(ref: 'master', environment: nil) do + [create(:ci_variable, key: 'secret', value: 'value')] + end + + allow_any_instance_of(Ci::Pipeline) + .to receive(:predefined_variables) { [pipeline_pre_var] } + end + + it 'returns variables in order depending on resource hierarchy' do + is_expected.to eq( + [build_pre_var, + project_pre_var, + pipeline_pre_var, + build_yaml_var, + { key: 'secret', value: 'value', public: false }]) + end + end + + context 'when build has environment and user-provided variables' do + let(:expected_variables) do + predefined_variables.map { |variable| variable.fetch(:key) } + + %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG + CI_ENVIRONMENT_URL] + end + + before do + create(:environment, project: build.project, + name: 'staging') + + build.yaml_variables = [{ key: 'YAML_VARIABLE', + value: 'var', + public: true }] + build.environment = 'staging' + end + + it 'matches explicit variables ordering' do + received_variables = subject.map { |variable| variable.fetch(:key) } + + expect(received_variables).to eq expected_variables + end + end + end end describe 'state transition: any => [:pending]' do From c9d2fd3cffbe3c1d85de05d9d26bcabd19676256 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 7 Mar 2018 12:39:53 +0100 Subject: [PATCH 2/4] Improve pipeline tests for variables to test ordering --- spec/models/ci/pipeline_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 14d234f6aab..86bb2fefae1 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -172,10 +172,10 @@ describe Ci::Pipeline, :mailer do it { is_expected.to be_an(Array) } - it 'includes the defined keys' do - keys = subject.map { |v| v[:key] } + it 'includes all predefined variables in a valid order' do + keys = subject.map { |variable| variable.fetch(:key) } - expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE') + expect(keys).to eq %w[CI_PIPELINE_ID CI_CONFIG_PATH CI_PIPELINE_SOURCE] end end From 5ceb439d211caccfe791bef968ba4a6e1bf38a4b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 7 Mar 2018 14:51:41 +0100 Subject: [PATCH 3/4] Remove enforcing uniqueness of build variables --- app/models/ci/build.rb | 2 +- spec/models/ci/build_spec.rb | 17 ++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 79e473c85b7..b230b7f47ef 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -268,7 +268,7 @@ module Ci variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule variables += persisted_environment_variables if environment - variables.reverse.uniq { |variable| variable.fetch(:key) }.reverse + variables end def features diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 692c4cc81e0..6e202de0db9 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1873,14 +1873,13 @@ describe Ci::Build do pipeline.variables.build(key: 'MYVAR', value: 'pipeline value') end - it 'removes duplicates and leaves the latest occurence' do - expect(subject.count { |variable| variable.fetch(:key) == 'MYVAR' }) - .to be 1 - end - it 'overrides YAML variable using a pipeline variable' do - is_expected.not_to include(key: 'MYVAR', value: 'myvar', public: true) - is_expected.to include(key: 'MYVAR', value: 'pipeline value', public: false) + variables = subject.reverse.uniq { |variable| variable[:key] }.reverse + + expect(variables) + .not_to include(key: 'MYVAR', value: 'myvar', public: true) + expect(variables) + .to include(key: 'MYVAR', value: 'pipeline value', public: false) end end @@ -1921,8 +1920,8 @@ describe Ci::Build do context 'when build has environment and user-provided variables' do let(:expected_variables) do predefined_variables.map { |variable| variable.fetch(:key) } + - %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG - CI_ENVIRONMENT_URL] + %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG + CI_ENVIRONMENT_URL] end before do From 2542d7e0a2f517eb739f2ac72a271a203aa4ad13 Mon Sep 17 00:00:00 2001 From: Jan Beckmann Date: Sun, 11 Mar 2018 20:02:55 +0100 Subject: [PATCH 4/4] Hide emoji popup after multiple spaces or blank lines Fixes #40620,#33678 --- app/assets/javascripts/gfm_auto_complete.js | 3 +-- changelogs/unreleased/fix-emoji-popup.yml | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/fix-emoji-popup.yml diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index 57a1fa107e5..28c04b36d45 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -131,9 +131,8 @@ class GfmAutoComplete { callbacks: { ...this.getDefaultCallbacks(), matcher(flag, subtext) { - const relevantText = subtext.trim().split(/\s/).pop(); const regexp = new RegExp(`(?:[^${glRegexp.unicodeLetters}0-9:]|\n|^):([^:]*)$`, 'gi'); - const match = regexp.exec(relevantText); + const match = regexp.exec(subtext); return match && match.length ? match[1] : null; }, diff --git a/changelogs/unreleased/fix-emoji-popup.yml b/changelogs/unreleased/fix-emoji-popup.yml new file mode 100644 index 00000000000..c81d061a5d7 --- /dev/null +++ b/changelogs/unreleased/fix-emoji-popup.yml @@ -0,0 +1,5 @@ +--- +title: Hide emoji popup after multiple spaces +merge_request: +author: Jan Beckmann +type: fixed