From 989131c530f06fc52e9212df1e3e8d48eae4902f Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 24 Dec 2015 14:43:07 +0100 Subject: Render milestone links as references --- .../filter/milestone_reference_filter_spec.rb | 73 ++++++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 spec/lib/banzai/filter/milestone_reference_filter_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb new file mode 100644 index 00000000000..c53e780d354 --- /dev/null +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe Banzai::Filter::MilestoneReferenceFilter, lib: true do + include FilterSpecHelper + + let(:project) { create(:project, :public) } + let(:milestone) { create(:milestone, project: project) } + + it 'requires project context' do + expect { described_class.call('') }.to raise_error(ArgumentError, /:project/) + end + + %w(pre code a style).each do |elem| + it "ignores valid references contained inside '#{elem}' element" do + exp = act = "<#{elem}>milestone #{milestone.to_reference}" + expect(reference_filter(act).to_html).to eq exp + end + end + + context 'internal reference' do + let(:reference) { milestone.to_reference } + + it 'links to a valid reference' do + doc = reference_filter("See #{reference}") + + expect(doc.css('a').first.attr('href')).to eq urls. + namespace_project_milestone_url(project.namespace, project, milestone) + end + + it 'links with adjacent text' do + doc = reference_filter("milestone (#{reference}.)") + expect(doc.to_html).to match(/\(<\/i> #{Regexp.escape(milestone.title)}<\/a>\.\)/) + end + + it 'includes a title attribute' do + doc = reference_filter("milestone #{reference}") + expect(doc.css('a').first.attr('title')).to eq "Milestone: #{milestone.title}" + end + + it 'escapes the title attribute' do + milestone.update_attribute(:title, %{">whatever Date: Mon, 28 Dec 2015 17:08:15 +0100 Subject: Fix spelling mistake, thanks Connor. --- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index c90133fbf03..d15100fc6d8 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' module Ci describe GitlabCiYamlProcessor, lib: true do let(:path) { 'path' } - + describe "#builds_for_ref" do let(:type) { 'test' } @@ -29,7 +29,7 @@ module Ci when: "on_success" }) end - + describe :only do it "does not return builds if only has another branch" do config = YAML.dump({ @@ -517,7 +517,7 @@ module Ci end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra") end - it "returns errors if there is no any jobs defined" do + it "returns errors if there are no jobs defined" do config = YAML.dump({ before_script: ["bundle update"] }) expect do GitlabCiYamlProcessor.new(config, path) -- cgit v1.2.1 From 03478e6d5b98a723fbb349dac2c8495f75909a08 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 29 Dec 2015 13:40:08 +0100 Subject: Strip newlines from obfuscated SQL Newlines aren't really needed and they may mess with InfluxDB's line protocol. --- spec/lib/gitlab/metrics/obfuscated_sql_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb b/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb index 0f01ee588c9..2b681c9fe34 100644 --- a/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb +++ b/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb @@ -2,6 +2,12 @@ require 'spec_helper' describe Gitlab::Metrics::ObfuscatedSQL do describe '#to_s' do + it 'replaces newlines with a space' do + sql = described_class.new("SELECT x\nFROM y") + + expect(sql.to_s).to eq('SELECT x FROM y') + end + describe 'using single values' do it 'replaces a single integer' do sql = described_class.new('SELECT x FROM y WHERE a = 10') -- cgit v1.2.1 From 620e7bb3d60c3685b494b26e256b793a47621da4 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 29 Dec 2015 13:40:42 +0100 Subject: Write to InfluxDB directly via UDP This removes the need for Sidekiq and any overhead/problems introduced by TCP. There are a few things to take into account: 1. When writing data to InfluxDB you may still get an error if the server becomes unavailable during the write. Because of this we're catching all exceptions and just ignore them (for now). 2. Writing via UDP apparently requires the timestamp to be in nanoseconds. Without this data either isn't written properly. 3. Due to the restrictions on UDP buffer sizes we're writing metrics one by one, instead of writing all of them at once. --- spec/lib/gitlab/metrics/sampler_spec.rb | 2 +- spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb | 8 ---- spec/lib/gitlab/metrics/transaction_spec.rb | 2 +- spec/lib/gitlab/metrics_spec.rb | 48 ++++++++++++++++++++++ 4 files changed, 50 insertions(+), 10 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/sampler_spec.rb b/spec/lib/gitlab/metrics/sampler_spec.rb index 69376c0b79b..51a941c48cd 100644 --- a/spec/lib/gitlab/metrics/sampler_spec.rb +++ b/spec/lib/gitlab/metrics/sampler_spec.rb @@ -38,7 +38,7 @@ describe Gitlab::Metrics::Sampler do describe '#flush' do it 'schedules the metrics using Sidekiq' do - expect(MetricsWorker).to receive(:perform_async). + expect(Gitlab::Metrics).to receive(:submit_metrics). with([an_instance_of(Hash)]) sampler.sample_memory_usage diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb index 05214efc565..5882e7d81c7 100644 --- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -11,14 +11,6 @@ describe Gitlab::Metrics::SidekiqMiddleware do middleware.call(worker, 'test', :test) { nil } end - - it 'does not track jobs of the MetricsWorker' do - worker = MetricsWorker.new - - expect(Gitlab::Metrics::Transaction).to_not receive(:new) - - middleware.call(worker, 'test', :test) { nil } - end end describe '#tag_worker' do diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index 5f17ff8ee75..6862fc9e2d1 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -68,7 +68,7 @@ describe Gitlab::Metrics::Transaction do it 'submits the metrics to Sidekiq' do transaction.track_self - expect(MetricsWorker).to receive(:perform_async). + expect(Gitlab::Metrics).to receive(:submit_metrics). with([an_instance_of(Hash)]) transaction.submit diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index 944642909aa..6c0682cac4d 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -33,4 +33,52 @@ describe Gitlab::Metrics do expect(file).to eq('spec/lib/gitlab/metrics_spec.rb') end end + + describe '#submit_metrics' do + it 'prepares and writes the metrics to InfluxDB' do + connection = double(:connection) + pool = double(:pool) + + expect(pool).to receive(:with).and_yield(connection) + expect(connection).to receive(:write_points).with(an_instance_of(Array)) + expect(Gitlab::Metrics).to receive(:pool).and_return(pool) + + described_class.submit_metrics([{ 'series' => 'kittens', 'tags' => {} }]) + end + end + + describe '#prepare_metrics' do + it 'returns a Hash with the keys as Symbols' do + metrics = described_class. + prepare_metrics([{ 'values' => {}, 'tags' => {} }]) + + expect(metrics).to eq([{ values: {}, tags: {} }]) + end + + it 'escapes tag values' do + metrics = described_class.prepare_metrics([ + { 'values' => {}, 'tags' => { 'foo' => 'bar=' } } + ]) + + expect(metrics).to eq([{ values: {}, tags: { 'foo' => 'bar\\=' } }]) + end + + it 'drops empty tags' do + metrics = described_class.prepare_metrics([ + { 'values' => {}, 'tags' => { 'cats' => '', 'dogs' => nil } } + ]) + + expect(metrics).to eq([{ values: {}, tags: {} }]) + end + end + + describe '#escape_value' do + it 'escapes an equals sign' do + expect(described_class.escape_value('foo=')).to eq('foo\\=') + end + + it 'casts values to Strings' do + expect(described_class.escape_value(10)).to eq('10') + end + end end -- cgit v1.2.1 From d9d2e8a3e8de62beff685457e4e305f93122b206 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 29 Dec 2015 21:58:16 -0500 Subject: Support a single directory traversal in RelativeLinkFilter Prior, if we were viewing a blob at `https://example.com/namespace/project/blob/master/doc/some-file.md` and it contained a relative link such as `[README](../README.md)`, the resulting link when viewing the blob would be: `https://example.com/namespace/project/blob/README.md` which omits the `master` ref, resulting in a 404. --- spec/lib/banzai/filter/relative_link_filter_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index 0b3e5ecbc9f..0e6685f0ffb 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb @@ -92,6 +92,14 @@ describe Banzai::Filter::RelativeLinkFilter, lib: true do to eq "/#{project_path}/blob/#{ref}/doc/api/README.md" end + it 'rebuilds relative URL for a file in the repository root' do + relative_link = link('../README.md') + doc = filter(relative_link, requested_path: 'doc/some-file.md') + + expect(doc.at_css('a')['href']). + to eq "/#{project_path}/blob/#{ref}/README.md" + end + it 'rebuilds relative URL for a file in the repo with an anchor' do doc = filter(link('README.md#section')) expect(doc.at_css('a')['href']). -- cgit v1.2.1 From c936e4e3c8282df17f2dc89b305233b7608003ca Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 30 Dec 2015 15:53:52 +0100 Subject: Removed various default metrics tags While it's useful to keep track of the different versions (Ruby, GitLab, etc) doing so for every point wastes disk space and possibly also RAM (which InfluxDB is all to eager to gobble up). If we want to see the performance differences between different GitLab versions simply looking at the performance since the last release date should suffice. --- spec/lib/gitlab/metrics/metric_spec.rb | 3 --- 1 file changed, 3 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/metric_spec.rb b/spec/lib/gitlab/metrics/metric_spec.rb index ec39bc9cce8..aa76315c79c 100644 --- a/spec/lib/gitlab/metrics/metric_spec.rb +++ b/spec/lib/gitlab/metrics/metric_spec.rb @@ -39,9 +39,6 @@ describe Gitlab::Metrics::Metric do expect(hash[:tags]).to be_an_instance_of(Hash) expect(hash[:tags][:hostname]).to be_an_instance_of(String) - expect(hash[:tags][:ruby_engine]).to be_an_instance_of(String) - expect(hash[:tags][:ruby_version]).to be_an_instance_of(String) - expect(hash[:tags][:gitlab_version]).to be_an_instance_of(String) expect(hash[:tags][:process_type]).to be_an_instance_of(String) end -- cgit v1.2.1 From 054df415f94abe1e517a729e53cdb325d592d31b Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 30 Dec 2015 18:16:53 +0100 Subject: Optimize CSS expressions produced by Nokogiri Nokogiri produces inefficient XPath expressions when given CSS expressions such as "a.gfm". Luckily these expressions can be optimized quite easily while still achieving the same results. In the two cases where this optimization is applied the run time has been reduced from around 170 ms to around 15 ms. --- spec/lib/banzai/querying_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 spec/lib/banzai/querying_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/banzai/querying_spec.rb b/spec/lib/banzai/querying_spec.rb new file mode 100644 index 00000000000..27da2a7439c --- /dev/null +++ b/spec/lib/banzai/querying_spec.rb @@ -0,0 +1,13 @@ +require 'spec_helper' + +describe Banzai::Querying do + describe '.css' do + it 'optimizes queries for elements with classes' do + document = double(:document) + + expect(document).to receive(:xpath).with(/^descendant::a/) + + described_class.css(document, 'a.gfm') + end + end +end -- cgit v1.2.1 From a6c60127e3e2966b2f29fa6e6e79503b130c2b02 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 31 Dec 2015 17:14:02 +0100 Subject: Removed tracking of raw SQL queries This particular setup had 3 problems: 1. Storing SQL queries as tags is very inefficient as InfluxDB ends up indexing every query (and they can get pretty large). Storing these as values instead means we can't always display the SQL as easily. 2. We already instrument ActiveRecord query methods, thus we already have timing information about database queries. 3. SQL obfuscation is difficult to get right and I'd rather not expose sensitive data by accident. --- spec/lib/gitlab/metrics/obfuscated_sql_spec.rb | 93 ---------------------- .../metrics/subscribers/active_record_spec.rb | 32 -------- 2 files changed, 125 deletions(-) delete mode 100644 spec/lib/gitlab/metrics/obfuscated_sql_spec.rb delete mode 100644 spec/lib/gitlab/metrics/subscribers/active_record_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb b/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb deleted file mode 100644 index 2b681c9fe34..00000000000 --- a/spec/lib/gitlab/metrics/obfuscated_sql_spec.rb +++ /dev/null @@ -1,93 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Metrics::ObfuscatedSQL do - describe '#to_s' do - it 'replaces newlines with a space' do - sql = described_class.new("SELECT x\nFROM y") - - expect(sql.to_s).to eq('SELECT x FROM y') - end - - describe 'using single values' do - it 'replaces a single integer' do - sql = described_class.new('SELECT x FROM y WHERE a = 10') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') - end - - it 'replaces a single float' do - sql = described_class.new('SELECT x FROM y WHERE a = 10.5') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') - end - - it 'replaces a single quoted string' do - sql = described_class.new("SELECT x FROM y WHERE a = 'foo'") - - expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') - end - - if Gitlab::Database.mysql? - it 'replaces a double quoted string' do - sql = described_class.new('SELECT x FROM y WHERE a = "foo"') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') - end - end - - it 'replaces a single regular expression' do - sql = described_class.new('SELECT x FROM y WHERE a = /foo/') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') - end - - it 'replaces regular expressions using escaped slashes' do - sql = described_class.new('SELECT x FROM y WHERE a = /foo\/bar/') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE a = ?') - end - end - - describe 'using consecutive values' do - it 'replaces multiple integers' do - sql = described_class.new('SELECT x FROM y WHERE z IN (10, 20, 30)') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (3 values)') - end - - it 'replaces multiple floats' do - sql = described_class.new('SELECT x FROM y WHERE z IN (1.5, 2.5, 3.5)') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (3 values)') - end - - it 'replaces multiple single quoted strings' do - sql = described_class.new("SELECT x FROM y WHERE z IN ('foo', 'bar')") - - expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (2 values)') - end - - if Gitlab::Database.mysql? - it 'replaces multiple double quoted strings' do - sql = described_class.new('SELECT x FROM y WHERE z IN ("foo", "bar")') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (2 values)') - end - end - - it 'replaces multiple regular expressions' do - sql = described_class.new('SELECT x FROM y WHERE z IN (/foo/, /bar/)') - - expect(sql.to_s).to eq('SELECT x FROM y WHERE z IN (2 values)') - end - end - - if Gitlab::Database.postgresql? - it 'replaces double quotes' do - sql = described_class.new('SELECT "x" FROM "y"') - - expect(sql.to_s).to eq('SELECT x FROM y') - end - end - end -end diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb deleted file mode 100644 index 05b6cc14716..00000000000 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Metrics::Subscribers::ActiveRecord do - let(:transaction) { Gitlab::Metrics::Transaction.new } - - let(:subscriber) { described_class.new } - - let(:event) do - double(:event, duration: 0.2, - payload: { sql: 'SELECT * FROM users WHERE id = 10' }) - end - - before do - allow(subscriber).to receive(:current_transaction).and_return(transaction) - - allow(Gitlab::Metrics).to receive(:last_relative_application_frame). - and_return(['app/models/foo.rb', 4]) - end - - describe '#sql' do - it 'tracks the execution of a SQL query' do - sql = 'SELECT * FROM users WHERE id = ?' - values = { duration: 0.2 } - tags = { sql: sql, file: 'app/models/foo.rb', line: 4 } - - expect(transaction).to receive(:add_metric). - with(described_class::SERIES, values, tags) - - subscriber.sql(event) - end - end -end -- cgit v1.2.1 From 55ed6e1c96e4072af81ed51c4377f2c015f237d5 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 31 Dec 2015 17:47:07 +0100 Subject: Cache InfluxDB settings after the first use This ensures we don't need to load anything from either PostgreSQL or the Rails cache whenever creating new InfluxDB connections. --- spec/lib/gitlab/metrics_spec.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index 6c0682cac4d..c2924708f44 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -1,15 +1,9 @@ require 'spec_helper' describe Gitlab::Metrics do - describe '.pool_size' do - it 'returns a Fixnum' do - expect(described_class.pool_size).to be_an_instance_of(Fixnum) - end - end - - describe '.timeout' do - it 'returns a Fixnum' do - expect(described_class.timeout).to be_an_instance_of(Fixnum) + describe '.settings' do + it 'returns a Hash' do + expect(described_class.settings).to be_an_instance_of(Hash) end end -- cgit v1.2.1 From bd9f86bb8abb4759a0c72f94fb0492b1ff8619b5 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 31 Dec 2015 17:51:12 +0100 Subject: Use separate series for Rails/Sidekiq transactions This removes the need for tagging all metrics with a "process_type" tag. --- spec/lib/gitlab/metrics/instrumentation_spec.rb | 2 +- spec/lib/gitlab/metrics/metric_spec.rb | 1 - spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb | 2 +- spec/lib/gitlab/metrics/subscribers/action_view_spec.rb | 2 +- spec/lib/gitlab/metrics/transaction_spec.rb | 4 ++-- 5 files changed, 5 insertions(+), 6 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb index a7eab9d11cc..a9003d8796b 100644 --- a/spec/lib/gitlab/metrics/instrumentation_spec.rb +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Metrics::Instrumentation do - let(:transaction) { Gitlab::Metrics::Transaction.new } + let(:transaction) { Gitlab::Metrics::Transaction.new('rspec') } before do @dummy = Class.new do diff --git a/spec/lib/gitlab/metrics/metric_spec.rb b/spec/lib/gitlab/metrics/metric_spec.rb index aa76315c79c..9b942855140 100644 --- a/spec/lib/gitlab/metrics/metric_spec.rb +++ b/spec/lib/gitlab/metrics/metric_spec.rb @@ -39,7 +39,6 @@ describe Gitlab::Metrics::Metric do expect(hash[:tags]).to be_an_instance_of(Hash) expect(hash[:tags][:hostname]).to be_an_instance_of(String) - expect(hash[:tags][:process_type]).to be_an_instance_of(String) end it 'includes the values' do diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb index 5882e7d81c7..5fda6de52f4 100644 --- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::Metrics::SidekiqMiddleware do describe '#tag_worker' do it 'adds the worker class and action to the transaction' do - trans = Gitlab::Metrics::Transaction.new + trans = Gitlab::Metrics::Transaction.new('rspec') worker = double(:worker, class: double(:class, name: 'TestWorker')) expect(trans).to receive(:add_tag).with(:action, 'TestWorker#perform') diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index c6cd584663f..bca76ca5a69 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Metrics::Subscribers::ActionView do - let(:transaction) { Gitlab::Metrics::Transaction.new } + let(:transaction) { Gitlab::Metrics::Transaction.new('rspec') } let(:subscriber) { described_class.new } diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index 6862fc9e2d1..345163bfbea 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Metrics::Transaction do - let(:transaction) { described_class.new } + let(:transaction) { described_class.new('rspec') } describe '#duration' do it 'returns the duration of a transaction in seconds' do @@ -58,7 +58,7 @@ describe Gitlab::Metrics::Transaction do describe '#track_self' do it 'adds a metric for the transaction itself' do expect(transaction).to receive(:add_metric). - with(described_class::SERIES, { duration: transaction.duration }, {}) + with('rspec', { duration: transaction.duration }, {}) transaction.track_self end -- cgit v1.2.1 From cafc784ee1d5d0a0279077272af8ee435bb110e4 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 31 Dec 2015 17:55:10 +0100 Subject: Removed tracking of hostnames for metrics This isn't hugely useful and mostly wastes InfluxDB space. We can re-add this whenever needed (but only once we really need it). --- spec/lib/gitlab/metrics/metric_spec.rb | 2 -- spec/lib/gitlab/metrics_spec.rb | 6 ------ 2 files changed, 8 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/metric_spec.rb b/spec/lib/gitlab/metrics/metric_spec.rb index 9b942855140..f718d536130 100644 --- a/spec/lib/gitlab/metrics/metric_spec.rb +++ b/spec/lib/gitlab/metrics/metric_spec.rb @@ -37,8 +37,6 @@ describe Gitlab::Metrics::Metric do it 'includes the tags' do expect(hash[:tags]).to be_an_instance_of(Hash) - - expect(hash[:tags][:hostname]).to be_an_instance_of(String) end it 'includes the values' do diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index c2924708f44..c2782f95c8e 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -13,12 +13,6 @@ describe Gitlab::Metrics do end end - describe '.hostname' do - it 'returns a String containing the hostname' do - expect(described_class.hostname).to eq(Socket.gethostname) - end - end - describe '.last_relative_application_frame' do it 'returns an Array containing a file path and line number' do file, line = described_class.last_relative_application_frame -- cgit v1.2.1 From 96075be6f4688a59335130dc796132ad4f232442 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Jan 2016 11:37:46 +0100 Subject: Ability to increment custom transaction values This will be used to store/increment the total query/view rendering timings on a per transaction basis. This in turn can greatly reduce the amount of metrics stored. --- spec/lib/gitlab/metrics/transaction_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index 345163bfbea..18d63bdbdb9 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -38,6 +38,18 @@ describe Gitlab::Metrics::Transaction do end end + describe '#increment' do + it 'increments a counter' do + transaction.increment(:time, 1) + transaction.increment(:time, 2) + + expect(transaction).to receive(:add_metric). + with('rspec', { duration: 0.0, time: 3 }, {}) + + transaction.track_self + end + end + describe '#add_tag' do it 'adds a tag' do transaction.add_tag(:foo, 'bar') -- cgit v1.2.1 From 66a997a91403eef62ffd9fb789e899619d021a26 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Jan 2016 12:14:36 +0100 Subject: Track total query/view timings in transactions --- .../gitlab/metrics/subscribers/action_view_spec.rb | 3 ++ .../metrics/subscribers/active_record_spec.rb | 35 ++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 spec/lib/gitlab/metrics/subscribers/active_record_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index bca76ca5a69..699d50f770a 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -28,6 +28,9 @@ describe Gitlab::Metrics::Subscribers::ActionView do line: 4 } + expect(transaction).to receive(:increment). + with(:view_duration, 2.1) + expect(transaction).to receive(:add_metric). with(described_class::SERIES, values, tags) diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb new file mode 100644 index 00000000000..9ecedd934c6 --- /dev/null +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +describe Gitlab::Metrics::Subscribers::ActiveRecord do + let(:transaction) { Gitlab::Metrics::Transaction.new('rspec') } + let(:subscriber) { described_class.new } + + let(:event) do + double(:event, duration: 0.2, + payload: { sql: 'SELECT * FROM users WHERE id = 10' }) + end + + describe '#sql' do + describe 'without a current transaction' do + it 'simply returns' do + expect_any_instance_of(Gitlab::Metrics::Transaction). + to_not receive(:increment) + + subscriber.sql(event) + end + end + + describe 'with a current transaction' do + it 'increments the :sql_duration value' do + expect(subscriber).to receive(:current_transaction). + at_least(:once). + and_return(transaction) + + expect(transaction).to receive(:increment). + with(:sql_duration, 0.2) + + subscriber.sql(event) + end + end + end +end -- cgit v1.2.1 From 825b46f8a3eb620f99192217d414b72dffe597d7 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Jan 2016 12:19:45 +0100 Subject: Track total method call times per transaction This makes it easier to see where time is spent without having to aggregate all the individual points in the method_calls series. --- spec/lib/gitlab/metrics/instrumentation_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb index a9003d8796b..af020f652be 100644 --- a/spec/lib/gitlab/metrics/instrumentation_spec.rb +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -48,6 +48,9 @@ describe Gitlab::Metrics::Instrumentation do allow(described_class).to receive(:transaction). and_return(transaction) + expect(transaction).to receive(:increment). + with(:method_duration, a_kind_of(Numeric)) + expect(transaction).to receive(:add_metric). with(described_class::SERIES, an_instance_of(Hash), method: 'Dummy.foo') @@ -102,6 +105,9 @@ describe Gitlab::Metrics::Instrumentation do allow(described_class).to receive(:transaction). and_return(transaction) + expect(transaction).to receive(:increment). + with(:method_duration, a_kind_of(Numeric)) + expect(transaction).to receive(:add_metric). with(described_class::SERIES, an_instance_of(Hash), method: 'Dummy#bar') -- cgit v1.2.1 From 2ea464bb272fa52ff34a188a921f0bc90811ca45 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Jan 2016 12:45:31 +0100 Subject: Use separate series for Rails/Sidekiq sample stats This removes the need for any tags to differentiate between Sidekiq and Rails statistics while still being able to separate the two. --- spec/lib/gitlab/metrics/sampler_spec.rb | 38 ++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 8 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/sampler_spec.rb b/spec/lib/gitlab/metrics/sampler_spec.rb index 51a941c48cd..27211350fbe 100644 --- a/spec/lib/gitlab/metrics/sampler_spec.rb +++ b/spec/lib/gitlab/metrics/sampler_spec.rb @@ -51,8 +51,8 @@ describe Gitlab::Metrics::Sampler do expect(Gitlab::Metrics::System).to receive(:memory_usage). and_return(9000) - expect(Gitlab::Metrics::Metric).to receive(:new). - with('memory_usage', value: 9000). + expect(sampler).to receive(:add_metric). + with(/memory_usage/, value: 9000). and_call_original sampler.sample_memory_usage @@ -64,8 +64,8 @@ describe Gitlab::Metrics::Sampler do expect(Gitlab::Metrics::System).to receive(:file_descriptor_count). and_return(4) - expect(Gitlab::Metrics::Metric).to receive(:new). - with('file_descriptors', value: 4). + expect(sampler).to receive(:add_metric). + with(/file_descriptors/, value: 4). and_call_original sampler.sample_file_descriptors @@ -74,8 +74,8 @@ describe Gitlab::Metrics::Sampler do describe '#sample_objects' do it 'adds a metric containing the amount of allocated objects' do - expect(Gitlab::Metrics::Metric).to receive(:new). - with('object_counts', an_instance_of(Hash), an_instance_of(Hash)). + expect(sampler).to receive(:add_metric). + with(/object_counts/, an_instance_of(Hash), an_instance_of(Hash)). at_least(:once). and_call_original @@ -87,11 +87,33 @@ describe Gitlab::Metrics::Sampler do it 'adds a metric containing garbage collection statistics' do expect(GC::Profiler).to receive(:total_time).and_return(0.24) - expect(Gitlab::Metrics::Metric).to receive(:new). - with('gc_statistics', an_instance_of(Hash)). + expect(sampler).to receive(:add_metric). + with(/gc_statistics/, an_instance_of(Hash)). and_call_original sampler.sample_gc end end + + describe '#add_metric' do + it 'prefixes the series name for a Rails process' do + expect(sampler).to receive(:sidekiq?).and_return(false) + + expect(Gitlab::Metrics::Metric).to receive(:new). + with('rails_cats', { value: 10 }, {}). + and_call_original + + sampler.add_metric('cats', value: 10) + end + + it 'prefixes the series name for a Sidekiq process' do + expect(sampler).to receive(:sidekiq?).and_return(true) + + expect(Gitlab::Metrics::Metric).to receive(:new). + with('sidekiq_cats', { value: 10 }, {}). + and_call_original + + sampler.add_metric('cats', value: 10) + end + end end -- cgit v1.2.1 From 2ee8f555996baca6b470d223ffad65419b730398 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Jan 2016 13:17:02 +0100 Subject: Automatically prefix transaction series names This ensures Rails and Sidekiq transactions are split into the series "rails_transactions" and "sidekiq_transactions" respectively. --- spec/lib/gitlab/metrics/instrumentation_spec.rb | 2 +- spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb | 2 +- spec/lib/gitlab/metrics/subscribers/action_view_spec.rb | 2 +- spec/lib/gitlab/metrics/subscribers/active_record_spec.rb | 2 +- spec/lib/gitlab/metrics/transaction_spec.rb | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/instrumentation_spec.rb b/spec/lib/gitlab/metrics/instrumentation_spec.rb index af020f652be..2a37cd40dde 100644 --- a/spec/lib/gitlab/metrics/instrumentation_spec.rb +++ b/spec/lib/gitlab/metrics/instrumentation_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Metrics::Instrumentation do - let(:transaction) { Gitlab::Metrics::Transaction.new('rspec') } + let(:transaction) { Gitlab::Metrics::Transaction.new } before do @dummy = Class.new do diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb index 5fda6de52f4..5882e7d81c7 100644 --- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -15,7 +15,7 @@ describe Gitlab::Metrics::SidekiqMiddleware do describe '#tag_worker' do it 'adds the worker class and action to the transaction' do - trans = Gitlab::Metrics::Transaction.new('rspec') + trans = Gitlab::Metrics::Transaction.new worker = double(:worker, class: double(:class, name: 'TestWorker')) expect(trans).to receive(:add_tag).with(:action, 'TestWorker#perform') diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index 699d50f770a..05e4fbbeb51 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Metrics::Subscribers::ActionView do - let(:transaction) { Gitlab::Metrics::Transaction.new('rspec') } + let(:transaction) { Gitlab::Metrics::Transaction.new } let(:subscriber) { described_class.new } diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index 9ecedd934c6..81f35ba5d40 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Metrics::Subscribers::ActiveRecord do - let(:transaction) { Gitlab::Metrics::Transaction.new('rspec') } + let(:transaction) { Gitlab::Metrics::Transaction.new } let(:subscriber) { described_class.new } let(:event) do diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index 18d63bdbdb9..b9b94947afa 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Metrics::Transaction do - let(:transaction) { described_class.new('rspec') } + let(:transaction) { described_class.new } describe '#duration' do it 'returns the duration of a transaction in seconds' do @@ -32,7 +32,7 @@ describe Gitlab::Metrics::Transaction do describe '#add_metric' do it 'adds a metric tagged with the transaction UUID' do expect(Gitlab::Metrics::Metric).to receive(:new). - with('foo', { number: 10 }, { transaction_id: transaction.uuid }) + with('rails_foo', { number: 10 }, { transaction_id: transaction.uuid }) transaction.add_metric('foo', number: 10) end @@ -44,7 +44,7 @@ describe Gitlab::Metrics::Transaction do transaction.increment(:time, 2) expect(transaction).to receive(:add_metric). - with('rspec', { duration: 0.0, time: 3 }, {}) + with('transactions', { duration: 0.0, time: 3 }, {}) transaction.track_self end @@ -70,7 +70,7 @@ describe Gitlab::Metrics::Transaction do describe '#track_self' do it 'adds a metric for the transaction itself' do expect(transaction).to receive(:add_metric). - with('rspec', { duration: transaction.duration }, {}) + with('transactions', { duration: transaction.duration }, {}) transaction.track_self end -- cgit v1.2.1 From 8de491a68fcb130d436d2c85c0fda900381875cf Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Jan 2016 14:21:39 +0100 Subject: Fix Rubocop styling in AR subscriber specs --- spec/lib/gitlab/metrics/subscribers/active_record_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index 81f35ba5d40..7bc070a4d09 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do let(:event) do double(:event, duration: 0.2, - payload: { sql: 'SELECT * FROM users WHERE id = 10' }) + payload: { sql: 'SELECT * FROM users WHERE id = 10' }) end describe '#sql' do -- cgit v1.2.1 From 0b661516324862506d5ec30c44cac704346a90ad Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 5 Jan 2016 16:45:53 +0100 Subject: Remove icon from milestone reference. --- spec/lib/banzai/filter/milestone_reference_filter_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index c53e780d354..86b71210100 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -29,7 +29,7 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do it 'links with adjacent text' do doc = reference_filter("milestone (#{reference}.)") - expect(doc.to_html).to match(/\(<\/i> #{Regexp.escape(milestone.title)}<\/a>\.\)/) + expect(doc.to_html).to match(/\(#{Regexp.escape(milestone.title)}<\/a>\.\)/) end it 'includes a title attribute' do @@ -41,7 +41,7 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do milestone.update_attribute(:title, %{">whatever Date: Wed, 23 Dec 2015 15:04:46 -0200 Subject: Refactoring GithubImport::Importer --- spec/lib/gitlab/github_import/comment_spec.rb | 80 +++++++++++ spec/lib/gitlab/github_import/pull_request_spec.rb | 153 +++++++++++++++++++++ 2 files changed, 233 insertions(+) create mode 100644 spec/lib/gitlab/github_import/comment_spec.rb create mode 100644 spec/lib/gitlab/github_import/pull_request_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/github_import/comment_spec.rb b/spec/lib/gitlab/github_import/comment_spec.rb new file mode 100644 index 00000000000..ff6b115574b --- /dev/null +++ b/spec/lib/gitlab/github_import/comment_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::Comment, lib: true do + let(:project) { create(:project) } + let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') } + let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') } + let(:updated_at) { DateTime.strptime('2014-03-03T18:58:10Z') } + let(:base_data) do + { + body: "I'm having a problem with this.", + user: octocat, + created_at: created_at, + updated_at: updated_at + } + end + + subject(:comment) { described_class.new(project, raw_data)} + + describe '#attributes' do + context 'when do not reference a portion of the diff' do + let(:raw_data) { OpenStruct.new(base_data) } + + it 'returns formatted attributes' do + expected = { + project: project, + note: "*Created by: octocat*\n\nI'm having a problem with this.", + commit_id: nil, + line_code: nil, + author_id: project.creator_id, + created_at: created_at, + updated_at: updated_at + } + + expect(comment.attributes).to eq(expected) + end + end + + context 'when on a portion of the diff' do + let(:diff_data) do + { + body: 'Great stuff', + commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e', + diff_hunk: '@@ -16,33 +16,40 @@ public class Connection : IConnection...', + path: 'file1.txt', + position: 1 + } + end + + let(:raw_data) { OpenStruct.new(base_data.merge(diff_data)) } + + it 'returns formatted attributes' do + expected = { + project: project, + note: "*Created by: octocat*\n\nGreat stuff", + commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e', + line_code: 'ce1be0ff4065a6e9415095c95f25f47a633cef2b_0_1', + author_id: project.creator_id, + created_at: created_at, + updated_at: updated_at + } + + expect(comment.attributes).to eq(expected) + end + end + + context 'when author is a GitLab user' do + let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) } + + it 'returns project#creator_id as author_id when is not a GitLab user' do + expect(comment.attributes.fetch(:author_id)).to eq project.creator_id + end + + it 'returns GitLab user id as author_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(comment.attributes.fetch(:author_id)).to eq gl_user.id + end + end + end +end diff --git a/spec/lib/gitlab/github_import/pull_request_spec.rb b/spec/lib/gitlab/github_import/pull_request_spec.rb new file mode 100644 index 00000000000..6ac32a78955 --- /dev/null +++ b/spec/lib/gitlab/github_import/pull_request_spec.rb @@ -0,0 +1,153 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::PullRequest, lib: true do + let(:project) { create(:project) } + let(:source_branch) { OpenStruct.new(ref: 'feature') } + let(:target_branch) { OpenStruct.new(ref: 'master') } + let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') } + let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } + let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } + let(:base_data) do + { + state: 'open', + title: 'New feature', + body: 'Please pull these awesome changes', + head: source_branch, + base: target_branch, + assignee: nil, + user: octocat, + created_at: created_at, + updated_at: updated_at, + closed_at: nil, + merged_at: nil + } + end + + subject(:pull_request) { described_class.new(project, raw_data)} + + describe '#attributes' do + context 'when pull request is open' do + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'open')) } + + it 'returns formatted attributes' do + expected = { + title: 'New feature', + description: "*Created by: octocat*\n\nPlease pull these awesome changes", + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master', + state: 'opened', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: updated_at + } + + expect(pull_request.attributes).to eq(expected) + end + end + + context 'when pull request is closed' do + let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') } + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', closed_at: closed_at)) } + + it 'returns formatted attributes' do + expected = { + title: 'New feature', + description: "*Created by: octocat*\n\nPlease pull these awesome changes", + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master', + state: 'closed', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: closed_at + } + + expect(pull_request.attributes).to eq(expected) + end + end + + context 'when pull request is merged' do + let(:merged_at) { DateTime.strptime('2011-01-28T13:01:12Z') } + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', merged_at: merged_at)) } + + it 'returns formatted attributes' do + expected = { + title: 'New feature', + description: "*Created by: octocat*\n\nPlease pull these awesome changes", + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master', + state: 'merged', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: merged_at + } + + expect(pull_request.attributes).to eq(expected) + end + end + + context 'when it is assigned to someone' do + let(:raw_data) { OpenStruct.new(base_data.merge(assignee: octocat)) } + + it 'returns nil as assigned_id when is not a GitLab user' do + expect(pull_request.attributes.fetch(:assignee_id)).to be_nil + end + + it 'returns GitLab user id as assigned_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(pull_request.attributes.fetch(:assignee_id)).to eq gl_user.id + end + end + + context 'when author is a GitLab user' do + let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) } + + it 'returns project#creator_id as author_id when is not a GitLab user' do + expect(pull_request.attributes.fetch(:author_id)).to eq project.creator_id + end + + it 'returns GitLab user id as author_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(pull_request.attributes.fetch(:author_id)).to eq gl_user.id + end + end + end + + describe '#valid?' do + let(:invalid_branch) { OpenStruct.new(ref: 'invalid-branch') } + + context 'when source and target branches exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: target_branch)) } + + it 'returns true' do + expect(pull_request.valid?).to eq true + end + end + + context 'when source branch doesn not exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: invalid_branch, base: target_branch)) } + + it 'returns false' do + expect(pull_request.valid?).to eq false + end + end + + context 'when target branch doesn not exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: invalid_branch)) } + + it 'returns false' do + expect(pull_request.valid?).to eq false + end + end + end +end -- cgit v1.2.1 From 98909dd12cd27b85921962326bcaf651c092dcd5 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 23 Dec 2015 20:02:59 -0200 Subject: Generate separate comments when importing GitHub Issues into GitLab --- .../gitlab/github_import/comment_formatter_spec.rb | 80 ++++++++++ spec/lib/gitlab/github_import/comment_spec.rb | 80 ---------- .../gitlab/github_import/issue_formatter_spec.rb | 139 ++++++++++++++++++ .../github_import/pull_request_formatter_spec.rb | 162 +++++++++++++++++++++ spec/lib/gitlab/github_import/pull_request_spec.rb | 153 ------------------- 5 files changed, 381 insertions(+), 233 deletions(-) create mode 100644 spec/lib/gitlab/github_import/comment_formatter_spec.rb delete mode 100644 spec/lib/gitlab/github_import/comment_spec.rb create mode 100644 spec/lib/gitlab/github_import/issue_formatter_spec.rb create mode 100644 spec/lib/gitlab/github_import/pull_request_formatter_spec.rb delete mode 100644 spec/lib/gitlab/github_import/pull_request_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/github_import/comment_formatter_spec.rb b/spec/lib/gitlab/github_import/comment_formatter_spec.rb new file mode 100644 index 00000000000..a324a82e69f --- /dev/null +++ b/spec/lib/gitlab/github_import/comment_formatter_spec.rb @@ -0,0 +1,80 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::CommentFormatter, lib: true do + let(:project) { create(:project) } + let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') } + let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') } + let(:updated_at) { DateTime.strptime('2014-03-03T18:58:10Z') } + let(:base_data) do + { + body: "I'm having a problem with this.", + user: octocat, + created_at: created_at, + updated_at: updated_at + } + end + + subject(:comment) { described_class.new(project, raw_data)} + + describe '#attributes' do + context 'when do not reference a portion of the diff' do + let(:raw_data) { OpenStruct.new(base_data) } + + it 'returns formatted attributes' do + expected = { + project: project, + note: "*Created by: octocat*\n\nI'm having a problem with this.", + commit_id: nil, + line_code: nil, + author_id: project.creator_id, + created_at: created_at, + updated_at: updated_at + } + + expect(comment.attributes).to eq(expected) + end + end + + context 'when on a portion of the diff' do + let(:diff_data) do + { + body: 'Great stuff', + commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e', + diff_hunk: '@@ -16,33 +16,40 @@ public class Connection : IConnection...', + path: 'file1.txt', + position: 1 + } + end + + let(:raw_data) { OpenStruct.new(base_data.merge(diff_data)) } + + it 'returns formatted attributes' do + expected = { + project: project, + note: "*Created by: octocat*\n\nGreat stuff", + commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e', + line_code: 'ce1be0ff4065a6e9415095c95f25f47a633cef2b_0_1', + author_id: project.creator_id, + created_at: created_at, + updated_at: updated_at + } + + expect(comment.attributes).to eq(expected) + end + end + + context 'when author is a GitLab user' do + let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) } + + it 'returns project#creator_id as author_id when is not a GitLab user' do + expect(comment.attributes.fetch(:author_id)).to eq project.creator_id + end + + it 'returns GitLab user id as author_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(comment.attributes.fetch(:author_id)).to eq gl_user.id + end + end + end +end diff --git a/spec/lib/gitlab/github_import/comment_spec.rb b/spec/lib/gitlab/github_import/comment_spec.rb deleted file mode 100644 index ff6b115574b..00000000000 --- a/spec/lib/gitlab/github_import/comment_spec.rb +++ /dev/null @@ -1,80 +0,0 @@ -require 'spec_helper' - -describe Gitlab::GithubImport::Comment, lib: true do - let(:project) { create(:project) } - let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') } - let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') } - let(:updated_at) { DateTime.strptime('2014-03-03T18:58:10Z') } - let(:base_data) do - { - body: "I'm having a problem with this.", - user: octocat, - created_at: created_at, - updated_at: updated_at - } - end - - subject(:comment) { described_class.new(project, raw_data)} - - describe '#attributes' do - context 'when do not reference a portion of the diff' do - let(:raw_data) { OpenStruct.new(base_data) } - - it 'returns formatted attributes' do - expected = { - project: project, - note: "*Created by: octocat*\n\nI'm having a problem with this.", - commit_id: nil, - line_code: nil, - author_id: project.creator_id, - created_at: created_at, - updated_at: updated_at - } - - expect(comment.attributes).to eq(expected) - end - end - - context 'when on a portion of the diff' do - let(:diff_data) do - { - body: 'Great stuff', - commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e', - diff_hunk: '@@ -16,33 +16,40 @@ public class Connection : IConnection...', - path: 'file1.txt', - position: 1 - } - end - - let(:raw_data) { OpenStruct.new(base_data.merge(diff_data)) } - - it 'returns formatted attributes' do - expected = { - project: project, - note: "*Created by: octocat*\n\nGreat stuff", - commit_id: '6dcb09b5b57875f334f61aebed695e2e4193db5e', - line_code: 'ce1be0ff4065a6e9415095c95f25f47a633cef2b_0_1', - author_id: project.creator_id, - created_at: created_at, - updated_at: updated_at - } - - expect(comment.attributes).to eq(expected) - end - end - - context 'when author is a GitLab user' do - let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) } - - it 'returns project#creator_id as author_id when is not a GitLab user' do - expect(comment.attributes.fetch(:author_id)).to eq project.creator_id - end - - it 'returns GitLab user id as author_id when is a GitLab user' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') - - expect(comment.attributes.fetch(:author_id)).to eq gl_user.id - end - end - end -end diff --git a/spec/lib/gitlab/github_import/issue_formatter_spec.rb b/spec/lib/gitlab/github_import/issue_formatter_spec.rb new file mode 100644 index 00000000000..fd05428b322 --- /dev/null +++ b/spec/lib/gitlab/github_import/issue_formatter_spec.rb @@ -0,0 +1,139 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::IssueFormatter, lib: true do + let!(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } + let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') } + let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } + let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } + + let(:base_data) do + { + number: 1347, + state: 'open', + title: 'Found a bug', + body: "I'm having a problem with this.", + assignee: nil, + user: octocat, + comments: 0, + pull_request: nil, + created_at: created_at, + updated_at: updated_at, + closed_at: nil + } + end + + subject(:issue) { described_class.new(project, raw_data)} + + describe '#attributes' do + context 'when issue is open' do + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'open')) } + + it 'returns formatted attributes' do + expected = { + project: project, + title: 'Found a bug', + description: "*Created by: octocat*\n\nI'm having a problem with this.", + state: 'opened', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: updated_at + } + + expect(issue.attributes).to eq(expected) + end + end + + context 'when issue is closed' do + let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') } + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', closed_at: closed_at)) } + + it 'returns formatted attributes' do + expected = { + project: project, + title: 'Found a bug', + description: "*Created by: octocat*\n\nI'm having a problem with this.", + state: 'closed', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: closed_at + } + + expect(issue.attributes).to eq(expected) + end + end + + context 'when it is assigned to someone' do + let(:raw_data) { OpenStruct.new(base_data.merge(assignee: octocat)) } + + it 'returns nil as assignee_id when is not a GitLab user' do + expect(issue.attributes.fetch(:assignee_id)).to be_nil + end + + it 'returns GitLab user id as assignee_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(issue.attributes.fetch(:assignee_id)).to eq gl_user.id + end + end + + context 'when author is a GitLab user' do + let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) } + + it 'returns project#creator_id as author_id when is not a GitLab user' do + expect(issue.attributes.fetch(:author_id)).to eq project.creator_id + end + + it 'returns GitLab user id as author_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(issue.attributes.fetch(:author_id)).to eq gl_user.id + end + end + end + + describe '#has_comments?' do + context 'when number of comments is greater than zero' do + let(:raw_data) { OpenStruct.new(base_data.merge(comments: 1)) } + + it 'returns true' do + expect(issue.has_comments?).to eq true + end + end + + context 'when number of comments is equal to zero' do + let(:raw_data) { OpenStruct.new(base_data.merge(comments: 0)) } + + it 'returns false' do + expect(issue.has_comments?).to eq false + end + end + end + + describe '#number' do + let(:raw_data) { OpenStruct.new(base_data.merge(number: 1347)) } + + it 'returns pull request number' do + expect(issue.number).to eq 1347 + end + end + + describe '#valid?' do + context 'when mention a pull request' do + let(:raw_data) { OpenStruct.new(base_data.merge(pull_request: OpenStruct.new)) } + + it 'returns false' do + expect(issue.valid?).to eq false + end + end + + context 'when does not mention a pull request' do + let(:raw_data) { OpenStruct.new(base_data.merge(pull_request: nil)) } + + it 'returns true' do + expect(issue.valid?).to eq true + end + end + end +end diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb new file mode 100644 index 00000000000..b4465ef3743 --- /dev/null +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -0,0 +1,162 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::PullRequestFormatter, lib: true do + let(:project) { create(:project) } + let(:source_branch) { OpenStruct.new(ref: 'feature') } + let(:target_branch) { OpenStruct.new(ref: 'master') } + let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') } + let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } + let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } + let(:base_data) do + { + number: 1347, + state: 'open', + title: 'New feature', + body: 'Please pull these awesome changes', + head: source_branch, + base: target_branch, + assignee: nil, + user: octocat, + created_at: created_at, + updated_at: updated_at, + closed_at: nil, + merged_at: nil + } + end + + subject(:pull_request) { described_class.new(project, raw_data)} + + describe '#attributes' do + context 'when pull request is open' do + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'open')) } + + it 'returns formatted attributes' do + expected = { + title: 'New feature', + description: "*Created by: octocat*\n\nPlease pull these awesome changes", + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master', + state: 'opened', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: updated_at + } + + expect(pull_request.attributes).to eq(expected) + end + end + + context 'when pull request is closed' do + let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') } + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', closed_at: closed_at)) } + + it 'returns formatted attributes' do + expected = { + title: 'New feature', + description: "*Created by: octocat*\n\nPlease pull these awesome changes", + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master', + state: 'closed', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: closed_at + } + + expect(pull_request.attributes).to eq(expected) + end + end + + context 'when pull request is merged' do + let(:merged_at) { DateTime.strptime('2011-01-28T13:01:12Z') } + let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', merged_at: merged_at)) } + + it 'returns formatted attributes' do + expected = { + title: 'New feature', + description: "*Created by: octocat*\n\nPlease pull these awesome changes", + source_project: project, + source_branch: 'feature', + target_project: project, + target_branch: 'master', + state: 'merged', + author_id: project.creator_id, + assignee_id: nil, + created_at: created_at, + updated_at: merged_at + } + + expect(pull_request.attributes).to eq(expected) + end + end + + context 'when it is assigned to someone' do + let(:raw_data) { OpenStruct.new(base_data.merge(assignee: octocat)) } + + it 'returns nil as assignee_id when is not a GitLab user' do + expect(pull_request.attributes.fetch(:assignee_id)).to be_nil + end + + it 'returns GitLab user id as assignee_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(pull_request.attributes.fetch(:assignee_id)).to eq gl_user.id + end + end + + context 'when author is a GitLab user' do + let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) } + + it 'returns project#creator_id as author_id when is not a GitLab user' do + expect(pull_request.attributes.fetch(:author_id)).to eq project.creator_id + end + + it 'returns GitLab user id as author_id when is a GitLab user' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(pull_request.attributes.fetch(:author_id)).to eq gl_user.id + end + end + end + + describe '#number' do + let(:raw_data) { OpenStruct.new(base_data.merge(number: 1347)) } + + it 'returns pull request number' do + expect(pull_request.number).to eq 1347 + end + end + + describe '#valid?' do + let(:invalid_branch) { OpenStruct.new(ref: 'invalid-branch') } + + context 'when source and target branches exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: target_branch)) } + + it 'returns true' do + expect(pull_request.valid?).to eq true + end + end + + context 'when source branch doesn not exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: invalid_branch, base: target_branch)) } + + it 'returns false' do + expect(pull_request.valid?).to eq false + end + end + + context 'when target branch doesn not exists' do + let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: invalid_branch)) } + + it 'returns false' do + expect(pull_request.valid?).to eq false + end + end + end +end diff --git a/spec/lib/gitlab/github_import/pull_request_spec.rb b/spec/lib/gitlab/github_import/pull_request_spec.rb deleted file mode 100644 index 6ac32a78955..00000000000 --- a/spec/lib/gitlab/github_import/pull_request_spec.rb +++ /dev/null @@ -1,153 +0,0 @@ -require 'spec_helper' - -describe Gitlab::GithubImport::PullRequest, lib: true do - let(:project) { create(:project) } - let(:source_branch) { OpenStruct.new(ref: 'feature') } - let(:target_branch) { OpenStruct.new(ref: 'master') } - let(:octocat) { OpenStruct.new(id: 123456, login: 'octocat') } - let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } - let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } - let(:base_data) do - { - state: 'open', - title: 'New feature', - body: 'Please pull these awesome changes', - head: source_branch, - base: target_branch, - assignee: nil, - user: octocat, - created_at: created_at, - updated_at: updated_at, - closed_at: nil, - merged_at: nil - } - end - - subject(:pull_request) { described_class.new(project, raw_data)} - - describe '#attributes' do - context 'when pull request is open' do - let(:raw_data) { OpenStruct.new(base_data.merge(state: 'open')) } - - it 'returns formatted attributes' do - expected = { - title: 'New feature', - description: "*Created by: octocat*\n\nPlease pull these awesome changes", - source_project: project, - source_branch: 'feature', - target_project: project, - target_branch: 'master', - state: 'opened', - author_id: project.creator_id, - assignee_id: nil, - created_at: created_at, - updated_at: updated_at - } - - expect(pull_request.attributes).to eq(expected) - end - end - - context 'when pull request is closed' do - let(:closed_at) { DateTime.strptime('2011-01-28T19:01:12Z') } - let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', closed_at: closed_at)) } - - it 'returns formatted attributes' do - expected = { - title: 'New feature', - description: "*Created by: octocat*\n\nPlease pull these awesome changes", - source_project: project, - source_branch: 'feature', - target_project: project, - target_branch: 'master', - state: 'closed', - author_id: project.creator_id, - assignee_id: nil, - created_at: created_at, - updated_at: closed_at - } - - expect(pull_request.attributes).to eq(expected) - end - end - - context 'when pull request is merged' do - let(:merged_at) { DateTime.strptime('2011-01-28T13:01:12Z') } - let(:raw_data) { OpenStruct.new(base_data.merge(state: 'closed', merged_at: merged_at)) } - - it 'returns formatted attributes' do - expected = { - title: 'New feature', - description: "*Created by: octocat*\n\nPlease pull these awesome changes", - source_project: project, - source_branch: 'feature', - target_project: project, - target_branch: 'master', - state: 'merged', - author_id: project.creator_id, - assignee_id: nil, - created_at: created_at, - updated_at: merged_at - } - - expect(pull_request.attributes).to eq(expected) - end - end - - context 'when it is assigned to someone' do - let(:raw_data) { OpenStruct.new(base_data.merge(assignee: octocat)) } - - it 'returns nil as assigned_id when is not a GitLab user' do - expect(pull_request.attributes.fetch(:assignee_id)).to be_nil - end - - it 'returns GitLab user id as assigned_id when is a GitLab user' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') - - expect(pull_request.attributes.fetch(:assignee_id)).to eq gl_user.id - end - end - - context 'when author is a GitLab user' do - let(:raw_data) { OpenStruct.new(base_data.merge(user: octocat)) } - - it 'returns project#creator_id as author_id when is not a GitLab user' do - expect(pull_request.attributes.fetch(:author_id)).to eq project.creator_id - end - - it 'returns GitLab user id as author_id when is a GitLab user' do - gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') - - expect(pull_request.attributes.fetch(:author_id)).to eq gl_user.id - end - end - end - - describe '#valid?' do - let(:invalid_branch) { OpenStruct.new(ref: 'invalid-branch') } - - context 'when source and target branches exists' do - let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: target_branch)) } - - it 'returns true' do - expect(pull_request.valid?).to eq true - end - end - - context 'when source branch doesn not exists' do - let(:raw_data) { OpenStruct.new(base_data.merge(head: invalid_branch, base: target_branch)) } - - it 'returns false' do - expect(pull_request.valid?).to eq false - end - end - - context 'when target branch doesn not exists' do - let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch, base: invalid_branch)) } - - it 'returns false' do - expect(pull_request.valid?).to eq false - end - end - end -end -- cgit v1.2.1 From 837a9065f0ff192d2efd55edcc2658a92c127b21 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 5 Jan 2016 15:15:36 -0200 Subject: Ensure that we're only importing local pull requests --- .../github_import/pull_request_formatter_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index b4465ef3743..9aefec77f6d 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -124,6 +124,28 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end end + describe '#cross_project?' do + context 'when source repo is not a fork' do + let(:local_repo) { OpenStruct.new(fork: false) } + let(:source_branch) { OpenStruct.new(ref: 'feature', repo: local_repo) } + let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch)) } + + it 'returns false' do + expect(pull_request.cross_project?).to eq false + end + end + + context 'when source repo is a fork' do + let(:forked_repo) { OpenStruct.new(fork: true) } + let(:source_branch) { OpenStruct.new(ref: 'feature', repo: forked_repo) } + let(:raw_data) { OpenStruct.new(base_data.merge(head: source_branch)) } + + it 'returns true' do + expect(pull_request.cross_project?).to eq true + end + end + end + describe '#number' do let(:raw_data) { OpenStruct.new(base_data.merge(number: 1347)) } -- cgit v1.2.1 From 7549102bb727daecc51da84af39956b32fc41537 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 6 Jan 2016 15:30:02 +0100 Subject: Store SQL/view timings in milliseconds Transaction timings are also already stored in milliseconds, this keeps things consistent. --- spec/lib/gitlab/metrics/subscribers/action_view_spec.rb | 4 ++-- spec/lib/gitlab/metrics/subscribers/active_record_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index 05e4fbbeb51..0a4cc5e929b 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::Metrics::Subscribers::ActionView do describe '#render_template' do it 'tracks rendering of a template' do - values = { duration: 2.1 } + values = { duration: 2100 } tags = { view: 'app/views/x.html.haml', file: 'app/views/x.html.haml', @@ -29,7 +29,7 @@ describe Gitlab::Metrics::Subscribers::ActionView do } expect(transaction).to receive(:increment). - with(:view_duration, 2.1) + with(:view_duration, 2100) expect(transaction).to receive(:add_metric). with(described_class::SERIES, values, tags) diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index 7bc070a4d09..ca86142a2f4 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -26,7 +26,7 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do and_return(transaction) expect(transaction).to receive(:increment). - with(:sql_duration, 0.2) + with(:sql_duration, 200) subscriber.sql(event) end -- cgit v1.2.1 From 7ed3a5a240e4997b24d11b96e27126dfaa575abe Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 7 Jan 2016 11:47:06 +0100 Subject: Revert "Store SQL/view timings in milliseconds" This reverts commit 7549102bb727daecc51da84af39956b32fc41537. Apparently I was wrong about ActiveSupport::Notifications::Event#duration returning the duration in seconds, instead it returns it in milliseconds already. --- spec/lib/gitlab/metrics/subscribers/action_view_spec.rb | 4 ++-- spec/lib/gitlab/metrics/subscribers/active_record_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index 0a4cc5e929b..05e4fbbeb51 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -21,7 +21,7 @@ describe Gitlab::Metrics::Subscribers::ActionView do describe '#render_template' do it 'tracks rendering of a template' do - values = { duration: 2100 } + values = { duration: 2.1 } tags = { view: 'app/views/x.html.haml', file: 'app/views/x.html.haml', @@ -29,7 +29,7 @@ describe Gitlab::Metrics::Subscribers::ActionView do } expect(transaction).to receive(:increment). - with(:view_duration, 2100) + with(:view_duration, 2.1) expect(transaction).to receive(:add_metric). with(described_class::SERIES, values, tags) diff --git a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb index ca86142a2f4..7bc070a4d09 100644 --- a/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/active_record_spec.rb @@ -26,7 +26,7 @@ describe Gitlab::Metrics::Subscribers::ActiveRecord do and_return(transaction) expect(transaction).to receive(:increment). - with(:sql_duration, 200) + with(:sql_duration, 0.2) subscriber.sql(event) end -- cgit v1.2.1 From 539b41929bddf0e82d986f9e823208dd92707a21 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 7 Jan 2016 12:26:05 +0100 Subject: Milestone reference is a Markdown link --- spec/lib/banzai/filter/milestone_reference_filter_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index 86b71210100..ebf3d7489b5 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -18,7 +18,9 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do end context 'internal reference' do - let(:reference) { milestone.to_reference } + # Convert the Markdown link to only the URL, since these tests aren't run through the regular Markdown pipeline. + # Milestone reference behavior in the full Markdown pipeline is tested elsewhere. + let(:reference) { milestone.to_reference.gsub(/\[([^\]]+)\]\(([^)]+)\)/, '\2') } it 'links to a valid reference' do doc = reference_filter("See #{reference}") -- cgit v1.2.1 From 364b07cff0183956ea11962b94c70448767351d3 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 7 Jan 2016 12:44:15 +0100 Subject: Removed UUIDs from metrics transactions While useful for finding out what methods/views belong to a transaction this might result in too much data being stored in InfluxDB. --- spec/lib/gitlab/metrics/transaction_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index b9b94947afa..0c98b8f0127 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -30,9 +30,9 @@ describe Gitlab::Metrics::Transaction do end describe '#add_metric' do - it 'adds a metric tagged with the transaction UUID' do + it 'adds a metric to the transaction' do expect(Gitlab::Metrics::Metric).to receive(:new). - with('rails_foo', { number: 10 }, { transaction_id: transaction.uuid }) + with('rails_foo', { number: 10 }, {}) transaction.add_metric('foo', number: 10) end -- cgit v1.2.1 From 7b10cb6f0f5758c17dd950587982ff400d7aa971 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 7 Jan 2016 13:05:00 +0100 Subject: Store request methods/URIs as values Since filtering by these values is very rare (they're mostly just displayed as-is) we don't need to waste any index space by saving them as tags. By storing them as values we also greatly reduce the number of series in InfluxDB. --- spec/lib/gitlab/metrics/rack_middleware_spec.rb | 6 +++--- spec/lib/gitlab/metrics/transaction_spec.rb | 11 +++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb index a143fe4cfcd..4e6dfc73df2 100644 --- a/spec/lib/gitlab/metrics/rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb @@ -40,9 +40,9 @@ describe Gitlab::Metrics::RackMiddleware do expect(transaction).to be_an_instance_of(Gitlab::Metrics::Transaction) end - it 'tags the transaction with the request method and URI' do - expect(transaction.tags[:request_method]).to eq('GET') - expect(transaction.tags[:request_uri]).to eq('/foo') + it 'stores the request method and URI in the transaction as values' do + expect(transaction.values[:request_method]).to eq('GET') + expect(transaction.values[:request_uri]).to eq('/foo') end end diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index 0c98b8f0127..3a27f897735 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -50,6 +50,17 @@ describe Gitlab::Metrics::Transaction do end end + describe '#set' do + it 'sets a value' do + transaction.set(:number, 10) + + expect(transaction).to receive(:add_metric). + with('transactions', { duration: 0.0, number: 10 }, {}) + + transaction.track_self + end + end + describe '#add_tag' do it 'adds a tag' do transaction.add_tag(:foo, 'bar') -- cgit v1.2.1 From 1e927d39b4bf6d1177dee0dd4a6c60bf270db3f2 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Thu, 7 Jan 2016 15:51:12 +0100 Subject: Update spec --- spec/lib/gitlab/email/receiver_spec.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb index b535413bbd4..abe179cd4af 100644 --- a/spec/lib/gitlab/email/receiver_spec.rb +++ b/spec/lib/gitlab/email/receiver_spec.rb @@ -42,7 +42,7 @@ describe Gitlab::Email::Receiver, lib: true do context "when the email was auto generated" do let!(:reply_key) { '636ca428858779856c226bb145ef4fad' } let!(:email_raw) { fixture_file("emails/auto_reply.eml") } - + it "raises an AutoGeneratedEmailError" do expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::AutoGeneratedEmailError) end @@ -90,7 +90,7 @@ describe Gitlab::Email::Receiver, lib: true do context "when the reply is blank" do let!(:email_raw) { fixture_file("emails/no_content_reply.eml") } - + it "raises an EmptyEmailError" do expect { receiver.execute }.to raise_error(Gitlab::Email::Receiver::EmptyEmailError) end @@ -107,13 +107,16 @@ describe Gitlab::Email::Receiver, lib: true do end context "when everything is fine" do + let(:markdown) { "![image](uploads/image.png)" } + before do allow_any_instance_of(Gitlab::Email::AttachmentUploader).to receive(:execute).and_return( [ { url: "uploads/image.png", is_image: true, - alt: "image" + alt: "image", + markdown: markdown } ] ) @@ -132,7 +135,7 @@ describe Gitlab::Email::Receiver, lib: true do note = noteable.notes.last - expect(note.note).to include("![image](uploads/image.png)") + expect(note.note).to include(markdown) end end end -- cgit v1.2.1 From 69209612e1793fcebcdb784074056d7a02b0f6f7 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 22 Dec 2015 12:15:06 -0800 Subject: Suppress e-mails on failed builds if allow_failure is set Every time I push to GitLab, I get > 2 emails saying a spec failed when I don't care about benchmarks and other specs that have `allow_failure` set to `true`. --- spec/lib/gitlab/build_data_builder_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/build_data_builder_spec.rb b/spec/lib/gitlab/build_data_builder_spec.rb index 839b30f1ff4..38be9448794 100644 --- a/spec/lib/gitlab/build_data_builder_spec.rb +++ b/spec/lib/gitlab/build_data_builder_spec.rb @@ -14,6 +14,7 @@ describe 'Gitlab::BuildDataBuilder' do it { expect(data[:tag]).to eq(build.tag) } it { expect(data[:build_id]).to eq(build.id) } it { expect(data[:build_status]).to eq(build.status) } + it { expect(data[:build_allow_failure]).to eq(false) } it { expect(data[:project_id]).to eq(build.project.id) } it { expect(data[:project_name]).to eq(build.project.name_with_namespace) } end -- cgit v1.2.1 From 65308a9c15cd371986999d38eb6f359d8f601687 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 7 Jan 2016 15:23:29 -0500 Subject: Add spec for single-item task lists --- spec/lib/banzai/filter/task_list_filter_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/banzai/filter/task_list_filter_spec.rb b/spec/lib/banzai/filter/task_list_filter_spec.rb index f2e3a44478d..569cbc885c7 100644 --- a/spec/lib/banzai/filter/task_list_filter_spec.rb +++ b/spec/lib/banzai/filter/task_list_filter_spec.rb @@ -7,4 +7,10 @@ describe Banzai::Filter::TaskListFilter, lib: true do exp = act = %(
  • Item
) expect(filter(act).to_html).to eq exp end + + it 'applies `task-list` to single-item task lists' do + act = filter('
  • [ ] Task 1
') + + expect(act.to_html).to start_with '
    ' + end end -- cgit v1.2.1 From d6dc088affeee4568e771e1d7894e0bcdb955af8 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 30 Dec 2015 20:56:26 -0200 Subject: LDAP synchronization block/unblock new states --- spec/lib/gitlab/ldap/access_spec.rb | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index a628d0c0157..f58d70e809c 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -13,64 +13,59 @@ describe Gitlab::LDAP::Access, lib: true do end it { is_expected.to be_falsey } - + it 'should block user in GitLab' do access.allowed? expect(user).to be_blocked + expect(user).to be_ldap_blocked end end context 'when the user is found' do before do - allow(Gitlab::LDAP::Person). - to receive(:find_by_dn).and_return(:ldap_user) + allow(Gitlab::LDAP::Person).to receive(:find_by_dn).and_return(:ldap_user) end context 'and the user is disabled via active directory' do before do - allow(Gitlab::LDAP::Person). - to receive(:disabled_via_active_directory?).and_return(true) + allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(true) end it { is_expected.to be_falsey } - it "should block user in GitLab" do + it 'should block user in GitLab' do access.allowed? expect(user).to be_blocked + expect(user).to be_ldap_blocked end end context 'and has no disabled flag in active diretory' do before do user.block - - allow(Gitlab::LDAP::Person). - to receive(:disabled_via_active_directory?).and_return(false) + allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false) end it { is_expected.to be_truthy } context 'when auto-created users are blocked' do - before do - allow_any_instance_of(Gitlab::LDAP::Config). - to receive(:block_auto_created_users).and_return(true) + allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(true) end - it "does not unblock user in GitLab" do + it 'does not unblock user in GitLab' do access.allowed? expect(user).to be_blocked + expect(user).not_to be_ldap_blocked # this block is handled by omniauth not by our internal logic end end - context "when auto-created users are not blocked" do - + context 'when auto-created users are not blocked' do before do - allow_any_instance_of(Gitlab::LDAP::Config). - to receive(:block_auto_created_users).and_return(false) + allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(false) end - it "should unblock user in GitLab" do + it 'should unblock user in GitLab' do access.allowed? expect(user).not_to be_blocked end @@ -80,8 +75,7 @@ describe Gitlab::LDAP::Access, lib: true do context 'without ActiveDirectory enabled' do before do allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true) - allow_any_instance_of(Gitlab::LDAP::Config). - to receive(:active_directory).and_return(false) + allow_any_instance_of(Gitlab::LDAP::Config).to receive(:active_directory).and_return(false) end it { is_expected.to be_truthy } -- cgit v1.2.1 From 35b501f30ae9e121151ad6a2140d036e5ef3b0f5 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 11 Jan 2016 16:51:01 +0100 Subject: Tag all transaction metrics with an "action" tag Without this it's impossible to find out what methods/views/queries are executed by a certain controller or Sidekiq worker. While this will increase the total number of series it should stay within reasonable limits due to the amount of "actions" being small enough. --- spec/lib/gitlab/metrics/rack_middleware_spec.rb | 2 +- spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb | 17 +++++------------ spec/lib/gitlab/metrics/transaction_spec.rb | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 13 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/rack_middleware_spec.rb b/spec/lib/gitlab/metrics/rack_middleware_spec.rb index 4e6dfc73df2..b99be4e1060 100644 --- a/spec/lib/gitlab/metrics/rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/rack_middleware_spec.rb @@ -57,7 +57,7 @@ describe Gitlab::Metrics::RackMiddleware do middleware.tag_controller(transaction, env) - expect(transaction.tags[:action]).to eq('TestController#show') + expect(transaction.action).to eq('TestController#show') end end end diff --git a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb index 5882e7d81c7..e520a968999 100644 --- a/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/sidekiq_middleware_spec.rb @@ -5,22 +5,15 @@ describe Gitlab::Metrics::SidekiqMiddleware do describe '#call' do it 'tracks the transaction' do - worker = Class.new.new + worker = double(:worker, class: double(:class, name: 'TestWorker')) + + expect(Gitlab::Metrics::Transaction).to receive(:new). + with('TestWorker#perform'). + and_call_original expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish) middleware.call(worker, 'test', :test) { nil } end end - - describe '#tag_worker' do - it 'adds the worker class and action to the transaction' do - trans = Gitlab::Metrics::Transaction.new - worker = double(:worker, class: double(:class, name: 'TestWorker')) - - expect(trans).to receive(:add_tag).with(:action, 'TestWorker#perform') - - middleware.tag_worker(trans, worker) - end - end end diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index 3a27f897735..6bdeb719491 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -96,5 +96,22 @@ describe Gitlab::Metrics::Transaction do transaction.submit end + + it 'adds the action as a tag for every metric' do + transaction.action = 'Foo#bar' + transaction.track_self + + hash = { + series: 'rails_transactions', + tags: { action: 'Foo#bar' }, + values: { duration: 0.0 }, + timestamp: an_instance_of(Fixnum) + } + + expect(Gitlab::Metrics).to receive(:submit_metrics). + with([hash]) + + transaction.submit + end end end -- cgit v1.2.1 From 5679ee0120ab45829b847d55d3a2253735856b3f Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 12 Jan 2016 14:59:30 +0100 Subject: Track memory allocated during a transaction This gives a very rough estimate of how much memory is allocated during a transaction. This only works reliably when using a single-threaded application server and a Ruby implementation with a GIL as otherwise memory allocated by other threads might skew the statistics. Sadly there's no way around this as Ruby doesn't provide a reliable way of gathering accurate object sizes upon allocation on a per-thread basis. --- spec/lib/gitlab/metrics/transaction_spec.rb | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb index 6bdeb719491..1d5a51a157e 100644 --- a/spec/lib/gitlab/metrics/transaction_spec.rb +++ b/spec/lib/gitlab/metrics/transaction_spec.rb @@ -11,6 +11,14 @@ describe Gitlab::Metrics::Transaction do end end + describe '#allocated_memory' do + it 'returns the allocated memory in bytes' do + transaction.run { 'a' * 32 } + + expect(transaction.allocated_memory).to be_a_kind_of(Numeric) + end + end + describe '#run' do it 'yields the supplied block' do expect { |b| transaction.run(&b) }.to yield_control @@ -43,8 +51,10 @@ describe Gitlab::Metrics::Transaction do transaction.increment(:time, 1) transaction.increment(:time, 2) + values = { duration: 0.0, time: 3, allocated_memory: a_kind_of(Numeric) } + expect(transaction).to receive(:add_metric). - with('transactions', { duration: 0.0, time: 3 }, {}) + with('transactions', values, {}) transaction.track_self end @@ -54,8 +64,14 @@ describe Gitlab::Metrics::Transaction do it 'sets a value' do transaction.set(:number, 10) + values = { + duration: 0.0, + number: 10, + allocated_memory: a_kind_of(Numeric) + } + expect(transaction).to receive(:add_metric). - with('transactions', { duration: 0.0, number: 10 }, {}) + with('transactions', values, {}) transaction.track_self end @@ -80,8 +96,13 @@ describe Gitlab::Metrics::Transaction do describe '#track_self' do it 'adds a metric for the transaction itself' do + values = { + duration: transaction.duration, + allocated_memory: a_kind_of(Numeric) + } + expect(transaction).to receive(:add_metric). - with('transactions', { duration: transaction.duration }, {}) + with('transactions', values, {}) transaction.track_self end @@ -104,7 +125,7 @@ describe Gitlab::Metrics::Transaction do hash = { series: 'rails_transactions', tags: { action: 'Foo#bar' }, - values: { duration: 0.0 }, + values: { duration: 0.0, allocated_memory: a_kind_of(Numeric) }, timestamp: an_instance_of(Fixnum) } -- cgit v1.2.1 From 355c341fe7c6b7c503386cf0b0e39cc02dbc8902 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 12 Jan 2016 15:41:22 +0100 Subject: Stop tracking call stacks for instrumented views Where a vew is called from doesn't matter as much. We already know what action they belong to and this is more than enough information. By removing the file/line number from the list of tags we should also be able to reduce the number of series stored in InfluxDB. --- spec/lib/gitlab/metrics/subscribers/action_view_spec.rb | 9 +-------- spec/lib/gitlab/metrics_spec.rb | 9 --------- 2 files changed, 1 insertion(+), 17 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb index 05e4fbbeb51..0695c5ce096 100644 --- a/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/action_view_spec.rb @@ -14,19 +14,12 @@ describe Gitlab::Metrics::Subscribers::ActionView do before do allow(subscriber).to receive(:current_transaction).and_return(transaction) - - allow(Gitlab::Metrics).to receive(:last_relative_application_frame). - and_return(['app/views/x.html.haml', 4]) end describe '#render_template' do it 'tracks rendering of a template' do values = { duration: 2.1 } - tags = { - view: 'app/views/x.html.haml', - file: 'app/views/x.html.haml', - line: 4 - } + tags = { view: 'app/views/x.html.haml' } expect(transaction).to receive(:increment). with(:view_duration, 2.1) diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index c2782f95c8e..0ec8a6dc5cb 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -13,15 +13,6 @@ describe Gitlab::Metrics do end end - describe '.last_relative_application_frame' do - it 'returns an Array containing a file path and line number' do - file, line = described_class.last_relative_application_frame - - expect(line).to eq(__LINE__ - 2) - expect(file).to eq('spec/lib/gitlab/metrics_spec.rb') - end - end - describe '#submit_metrics' do it 'prepares and writes the metrics to InfluxDB' do connection = double(:connection) -- cgit v1.2.1 From 057eb824b5d7f38547506303dc80da6164715420 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 13 Jan 2016 12:57:46 +0100 Subject: Randomize metrics sample intervals Sampling data at a fixed interval means we can potentially miss data from events occurring between sampling intervals. For example, say we sample data every 15 seconds but Unicorn workers get killed after 10 seconds. In this particular case it's possible to miss interesting data as the sampler will never get to actually submitting data. To work around this (at least for the most part) the sampling interval is randomized as following: 1. Take the user specified sampling interval (15 seconds by default) 2. Divide it by 2 (referred to as "half" below) 3. Generate a range (using a step of 0.1) from -"half" to "half" 4. Every time the sampler goes to sleep we'll grab the user provided interval and add a randomly chosen "adjustment" to it while making sure we don't pick the same value twice in a row. For a specified timeout of 15 this means the actual intervals can be anywhere between 7.5 and 22.5, but never can the same interval be used twice in a row. The rationale behind this change is that on dev.gitlab.org I'm sometimes seeing certain Gitlab::Git/Rugged objects being retained, but only for a few minutes every 24 hours. Knowing the code of Gitlab and how much memory it uses/leaks I suspect we're missing data due to workers getting terminated before the sampler can write its data to InfluxDB. --- spec/lib/gitlab/metrics/sampler_spec.rb | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/metrics/sampler_spec.rb b/spec/lib/gitlab/metrics/sampler_spec.rb index 27211350fbe..38da77adc9f 100644 --- a/spec/lib/gitlab/metrics/sampler_spec.rb +++ b/spec/lib/gitlab/metrics/sampler_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::Metrics::Sampler do describe '#start' do it 'gathers a sample at a given interval' do - expect(sampler).to receive(:sleep).with(5) + expect(sampler).to receive(:sleep).with(a_kind_of(Numeric)) expect(sampler).to receive(:sample) expect(sampler).to receive(:loop).and_yield @@ -116,4 +116,24 @@ describe Gitlab::Metrics::Sampler do sampler.add_metric('cats', value: 10) end end + + describe '#sleep_interval' do + it 'returns a Numeric' do + expect(sampler.sleep_interval).to be_a_kind_of(Numeric) + end + + # Testing random behaviour is very hard, so treat this test as a basic smoke + # test instead of a very accurate behaviour/unit test. + it 'does not return the same interval twice in a row' do + last = nil + + 100.times do + interval = sampler.sleep_interval + + expect(interval).to_not eq(last) + + last = interval + end + end + end end -- cgit v1.2.1 From dd6fc01ff8a073880b67a323a547edeb5d63f167 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Thu, 14 Jan 2016 03:31:27 -0200 Subject: fixed LDAP activation on login to use new ldap_blocked state --- spec/lib/gitlab/ldap/access_spec.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index f58d70e809c..32a19bf344b 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -42,7 +42,6 @@ describe Gitlab::LDAP::Access, lib: true do context 'and has no disabled flag in active diretory' do before do - user.block allow(Gitlab::LDAP::Person).to receive(:disabled_via_active_directory?).and_return(false) end @@ -50,7 +49,7 @@ describe Gitlab::LDAP::Access, lib: true do context 'when auto-created users are blocked' do before do - allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(true) + user.block end it 'does not unblock user in GitLab' do @@ -62,7 +61,7 @@ describe Gitlab::LDAP::Access, lib: true do context 'when auto-created users are not blocked' do before do - allow_any_instance_of(Gitlab::LDAP::Config).to receive(:block_auto_created_users).and_return(false) + user.ldap_block end it 'should unblock user in GitLab' do -- cgit v1.2.1 From f5d530865875440d69217cf249715bffaa3d11b8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 Dec 2015 11:59:10 +0100 Subject: Add implementation of StringPath class `StringPath` class is something similar to Ruby's `Pathname` class, but does not involve any IO operations. `StringPath` objects require passing string representation of path, and array of paths that represents universe to constructor to be intantiated. --- spec/lib/gitlab/string_path_spec.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 spec/lib/gitlab/string_path_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb new file mode 100644 index 00000000000..14a08bcb49b --- /dev/null +++ b/spec/lib/gitlab/string_path_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +describe Gitlab::StringPath do + let(:universe) do + ['path/dir_1/', + 'path/dir_1/file_1', + 'path/second_dir', + 'path/second_dir/dir_3/file_2', + 'path/second_dir/dir_3/file_3', + 'another_file', + '/file/with/absolute_path'] + end + + describe '/file/with/absolute_path' do + subject { described_class.new('/file/with/absolute_path', universe) } + + it { is_expected.to be_absolute } + it { is_expected.to_not be_relative } + it { is_expected.to be_file } + end +end -- cgit v1.2.1 From 73d2c7a553ca239cdce04af793992fd579ad3e4b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 18 Dec 2015 12:32:21 +0100 Subject: Add new methods to StringPath --- spec/lib/gitlab/string_path_spec.rb | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 14a08bcb49b..6e75e1f3ced 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -2,8 +2,10 @@ require 'spec_helper' describe Gitlab::StringPath do let(:universe) do - ['path/dir_1/', + ['path/', + 'path/dir_1/', 'path/dir_1/file_1', + 'path/dir_1/file_b', 'path/second_dir', 'path/second_dir/dir_3/file_2', 'path/second_dir/dir_3/file_3', @@ -17,5 +19,34 @@ describe Gitlab::StringPath do it { is_expected.to be_absolute } it { is_expected.to_not be_relative } it { is_expected.to be_file } + + describe '#basename' do + subject { described_class.new('/file/with/absolute_path', universe).basename } + + it { is_expected.to eq 'absolute_path' } + end + end + + describe 'path/' do + subject { described_class.new('path/', universe) } + + it { is_expected.to be_directory } + it { is_expected.to be_relative } + end + + describe 'path/dir_1/' do + describe '#files' do + subject { described_class.new('path/dir_1/', universe).files } + + pending { is_expected.to all(be_an_instance_of described_class) } + pending { is_expected.to be eq [Gitlab::StringPath.new('path/dir_1/file_1', universe), + Gitlab::StringPath.new('path/dir_1/file_b', universe)] } + end + + describe '#basename' do + subject { described_class.new('path/dir_1/', universe).basename } + + it { is_expected.to eq 'dir_1/' } + end end end -- cgit v1.2.1 From 518b206287318006f9b57382a747b1474b6795a4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 19 Dec 2015 09:31:52 +0100 Subject: Add `parent` iteration implementation to `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 6e75e1f3ced..86e48f6ee0b 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -19,6 +19,7 @@ describe Gitlab::StringPath do it { is_expected.to be_absolute } it { is_expected.to_not be_relative } it { is_expected.to be_file } + it { is_expected.to_not have_parent } describe '#basename' do subject { described_class.new('/file/with/absolute_path', universe).basename } @@ -32,9 +33,13 @@ describe Gitlab::StringPath do it { is_expected.to be_directory } it { is_expected.to be_relative } + it { is_expected.to_not have_parent } end describe 'path/dir_1/' do + subject { described_class.new('path/dir_1/', universe) } + it { is_expected.to have_parent } + describe '#files' do subject { described_class.new('path/dir_1/', universe).files } @@ -45,8 +50,12 @@ describe Gitlab::StringPath do describe '#basename' do subject { described_class.new('path/dir_1/', universe).basename } - it { is_expected.to eq 'dir_1/' } end + + describe '#parent' do + subject { described_class.new('path/dir_1/', universe).parent } + it { is_expected.to eq Gitlab::StringPath.new('path/', universe) } + end end end -- cgit v1.2.1 From c184eeb8489a389bf9f3528f7fe012d1edf132cb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 21 Dec 2015 09:17:43 +0100 Subject: Improve `StringPath` specs (DRY) --- spec/lib/gitlab/string_path_spec.rb | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 86e48f6ee0b..d086a011763 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -6,6 +6,8 @@ describe Gitlab::StringPath do 'path/dir_1/', 'path/dir_1/file_1', 'path/dir_1/file_b', + 'path/dir_1/subdir/', + 'path/dir_1/subdir/subfile', 'path/second_dir', 'path/second_dir/dir_3/file_2', 'path/second_dir/dir_3/file_3', @@ -13,8 +15,12 @@ describe Gitlab::StringPath do '/file/with/absolute_path'] end - describe '/file/with/absolute_path' do - subject { described_class.new('/file/with/absolute_path', universe) } + def path(example) + described_class.new(example.metadata[:path], universe) + end + + describe '/file/with/absolute_path', path: '/file/with/absolute_path' do + subject { |example| path(example) } it { is_expected.to be_absolute } it { is_expected.to_not be_relative } @@ -22,26 +28,27 @@ describe Gitlab::StringPath do it { is_expected.to_not have_parent } describe '#basename' do - subject { described_class.new('/file/with/absolute_path', universe).basename } + subject { |example| path(example).basename } it { is_expected.to eq 'absolute_path' } end end - describe 'path/' do - subject { described_class.new('path/', universe) } + describe 'path/', path: 'path/' do + subject { |example| path(example) } it { is_expected.to be_directory } it { is_expected.to be_relative } it { is_expected.to_not have_parent } end - describe 'path/dir_1/' do - subject { described_class.new('path/dir_1/', universe) } + describe 'path/dir_1/', path: 'path/dir_1/' do + subject { |example| path(example) } + it { is_expected.to have_parent } describe '#files' do - subject { described_class.new('path/dir_1/', universe).files } + subject { |example| path(example).files } pending { is_expected.to all(be_an_instance_of described_class) } pending { is_expected.to be eq [Gitlab::StringPath.new('path/dir_1/file_1', universe), @@ -49,12 +56,14 @@ describe Gitlab::StringPath do end describe '#basename' do - subject { described_class.new('path/dir_1/', universe).basename } + subject { |example| path(example).basename } + it { is_expected.to eq 'dir_1/' } end describe '#parent' do - subject { described_class.new('path/dir_1/', universe).parent } + subject { |example| path(example).parent } + it { is_expected.to eq Gitlab::StringPath.new('path/', universe) } end end -- cgit v1.2.1 From d382335dcd9285c9355ed04dc12c5314bca3c024 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 21 Dec 2015 10:11:15 +0100 Subject: Add implementation of remaining methods in `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 53 ++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 10 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index d086a011763..7818e340726 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -16,7 +16,11 @@ describe Gitlab::StringPath do end def path(example) - described_class.new(example.metadata[:path], universe) + string_path(example.metadata[:path]) + end + + def string_path(string_path) + described_class.new(string_path, universe) end describe '/file/with/absolute_path', path: '/file/with/absolute_path' do @@ -47,14 +51,6 @@ describe Gitlab::StringPath do it { is_expected.to have_parent } - describe '#files' do - subject { |example| path(example).files } - - pending { is_expected.to all(be_an_instance_of described_class) } - pending { is_expected.to be eq [Gitlab::StringPath.new('path/dir_1/file_1', universe), - Gitlab::StringPath.new('path/dir_1/file_b', universe)] } - end - describe '#basename' do subject { |example| path(example).basename } @@ -64,7 +60,44 @@ describe Gitlab::StringPath do describe '#parent' do subject { |example| path(example).parent } - it { is_expected.to eq Gitlab::StringPath.new('path/', universe) } + it { is_expected.to eq string_path('path/') } + end + + describe '#descendants' do + subject { |example| path(example).descendants } + + it { is_expected.to be_an_instance_of Array } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/'), + string_path('path/dir_1/subdir/subfile') } + end + + describe '#children' do + subject { |example| path(example).children } + + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/') } + end + + describe '#files' do + subject { |example| path(example).files } + + it { is_expected.to all(be_file) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b') } + end + + describe '#directories' do + subject { |example| path(example).directories } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } end end end -- cgit v1.2.1 From 37b2c5dd5521f25a7195e82538a0ffc528c3ec6d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 21 Dec 2015 12:08:04 +0100 Subject: Add support for root path for `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 7818e340726..7ee69c7d3cb 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -11,6 +11,7 @@ describe Gitlab::StringPath do 'path/second_dir', 'path/second_dir/dir_3/file_2', 'path/second_dir/dir_3/file_3', + 'another_directory/', 'another_file', '/file/with/absolute_path'] end @@ -30,6 +31,7 @@ describe Gitlab::StringPath do it { is_expected.to_not be_relative } it { is_expected.to be_file } it { is_expected.to_not have_parent } + it { is_expected.to_not have_descendants } describe '#basename' do subject { |example| path(example).basename } @@ -43,7 +45,7 @@ describe Gitlab::StringPath do it { is_expected.to be_directory } it { is_expected.to be_relative } - it { is_expected.to_not have_parent } + it { is_expected.to have_parent } end describe 'path/dir_1/', path: 'path/dir_1/' do @@ -100,4 +102,24 @@ describe Gitlab::StringPath do it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } end end + + describe './', path: './' do + subject { |example| path(example) } + + it { is_expected.to_not have_parent } + it { is_expected.to have_descendants } + + describe '#descendants' do + subject { |example| path(example).descendants } + + it { expect(subject.count).to eq universe.count - 1 } + it { is_expected.to_not include string_path('./') } + end + + describe '#children' do + subject { |example| path(example).children } + + it { expect(subject.count).to eq 3 } + end + end end -- cgit v1.2.1 From b19e958d86f5363057f006c8dbf9a8e8762618b9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 22 Dec 2015 11:05:22 +0100 Subject: Add support for parent directories in `StringPath` This support is not completed though, as parent directory that is first in collection returned by `directories!` is not iterable yet. --- spec/lib/gitlab/string_path_spec.rb | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 7ee69c7d3cb..c1722977576 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -50,18 +50,20 @@ describe Gitlab::StringPath do describe 'path/dir_1/', path: 'path/dir_1/' do subject { |example| path(example) } - it { is_expected.to have_parent } describe '#basename' do subject { |example| path(example).basename } - it { is_expected.to eq 'dir_1/' } end + describe '#name' do + subject { |example| path(example).name } + it { is_expected.to eq 'dir_1' } + end + describe '#parent' do subject { |example| path(example).parent } - it { is_expected.to eq string_path('path/') } end @@ -101,6 +103,15 @@ describe Gitlab::StringPath do it { is_expected.to all(be_an_instance_of described_class) } it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } end + + describe '#directories!' do + subject { |example| path(example).directories! } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/subdir/'), + string_path('path/dir_1/../') } + end end describe './', path: './' do @@ -118,7 +129,6 @@ describe Gitlab::StringPath do describe '#children' do subject { |example| path(example).children } - it { expect(subject.count).to eq 3 } end end -- cgit v1.2.1 From 304c39b6dc5878664c0dace4e3af6bdd2fa395f0 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 22 Dec 2015 13:56:54 +0100 Subject: Fix rubocop offenses in `StringPath` specs --- spec/lib/gitlab/string_path_spec.rb | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index c1722977576..8290aab7701 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -72,19 +72,23 @@ describe Gitlab::StringPath do it { is_expected.to be_an_instance_of Array } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/'), - string_path('path/dir_1/subdir/subfile') } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/'), + string_path('path/dir_1/subdir/subfile') + end end describe '#children' do subject { |example| path(example).children } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/') } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/') + end end describe '#files' do @@ -92,8 +96,10 @@ describe Gitlab::StringPath do it { is_expected.to all(be_file) } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b') } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b') + end end describe '#directories' do @@ -109,8 +115,10 @@ describe Gitlab::StringPath do it { is_expected.to all(be_directory) } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/dir_1/../') } + it do + is_expected.to contain_exactly string_path('path/dir_1/subdir/'), + string_path('path/dir_1/../') + end end end -- cgit v1.2.1 From 3de8a4620a70c886c815576dc0a30a745cbb8ccb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 31 Dec 2015 12:21:56 +0100 Subject: Parse artifacts metadata stored in JSON format --- spec/lib/gitlab/string_path_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 8290aab7701..af46f9754ac 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -140,4 +140,20 @@ describe Gitlab::StringPath do it { expect(subject.count).to eq 3 } end end + + describe '#metadata' do + let(:universe) do + ['path/', 'path/file1', 'path/file2'] + end + + let(:metadata) do + [{ name: '/path/'}, { name: '/path/file1' }, { name: '/path/file2' }] + end + + subject do + described_class.new('path/file1', universe, metadata).metadata[:name] + end + + it { is_expected.to eq '/path/file1' } + end end -- cgit v1.2.1 From df41148662142ce20a77b092665f48dd4dfa7bfb Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 2 Jan 2016 20:09:21 +0100 Subject: Improve path sanitization in `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index af46f9754ac..0ef2155cb9b 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -45,7 +45,6 @@ describe Gitlab::StringPath do it { is_expected.to be_directory } it { is_expected.to be_relative } - it { is_expected.to have_parent } end describe 'path/dir_1/', path: 'path/dir_1/' do -- cgit v1.2.1 From a7f99b67a0bf1160f41ebf4dc92c618eb13a7a10 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 13:08:49 +0100 Subject: Extract artifacts metadata implementation to separate class --- .../lib/gitlab/ci/build/artifacts/metadata_spec.rb | 76 ++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb new file mode 100644 index 00000000000..8c648be5f02 --- /dev/null +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Artifacts::Metadata do + def metadata(path = '') + described_class.new(metadata_file_path, path) + end + + let(:metadata_file_path) do + Rails.root + 'spec/fixtures/ci_build_artifacts_metadata.gz' + end + + context 'metadata file exists' do + describe '#exists?' do + subject { metadata.exists? } + it { is_expected.to be true } + end + + describe '#match! ./' do + subject { metadata('./').match! } + + it 'matches correct paths' do + expect(subject.first).to contain_exactly 'ci_artifacts.txt', + 'other_artifacts_0.1.2/', + 'rails_sample.jpg' + end + + it 'matches metadata for every path' do + expect(subject.last.count).to eq 3 + end + + it 'return Hashes for each metadata' do + expect(subject.last).to all(be_kind_of(Hash)) + end + end + + describe '#match! other_artifacts_0.1.2' do + subject { metadata('other_artifacts_0.1.2').match! } + + it 'matches correct paths' do + expect(subject.first). + to contain_exactly 'other_artifacts_0.1.2/doc_sample.txt', + 'other_artifacts_0.1.2/another-subdirectory/' + end + end + + describe '#match! other_artifacts_0.1.2/another-subdirectory' do + subject { metadata('other_artifacts_0.1.2/another-subdirectory/').match! } + + it 'matches correct paths' do + expect(subject.first). + to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', + 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' + end + end + + describe '#to_string_path' do + subject { metadata('').to_string_path } + it { is_expected.to be_an_instance_of(Gitlab::StringPath) } + end + end + + context 'metadata file does not exist' do + let(:metadata_file_path) { '' } + + describe '#exists?' do + subject { metadata.exists? } + it { is_expected.to be false } + end + + describe '#match!' do + it 'raises error' do + expect { metadata.match! }.to raise_error(StandardError, /Metadata file not found/) + end + end + end +end -- cgit v1.2.1 From f948c00757ca9529817c7368610b0c0d6734d48f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 13:40:42 +0100 Subject: Do not depend on universe when checking parent in `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 0ef2155cb9b..a54bf109c80 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -30,7 +30,7 @@ describe Gitlab::StringPath do it { is_expected.to be_absolute } it { is_expected.to_not be_relative } it { is_expected.to be_file } - it { is_expected.to_not have_parent } + it { is_expected.to have_parent } it { is_expected.to_not have_descendants } describe '#basename' do @@ -140,6 +140,21 @@ describe Gitlab::StringPath do end end + describe '#nodes', path: './' do + subject { |example| path(example).nodes } + it { is_expected.to eq 1 } + end + + describe '#nodes', path: './test' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + + describe '#nodes', path: './test/' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + describe '#metadata' do let(:universe) do ['path/', 'path/file1', 'path/file2'] -- cgit v1.2.1 From a5e1905d28e490fb4734bff0e02a1ecff4c7c029 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 14:00:49 +0100 Subject: Render 404 when artifacts path is invalid --- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 8c648be5f02..62c86a60ac4 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -38,7 +38,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do it 'matches correct paths' do expect(subject.first). - to contain_exactly 'other_artifacts_0.1.2/doc_sample.txt', + to contain_exactly 'other_artifacts_0.1.2/', + 'other_artifacts_0.1.2/doc_sample.txt', 'other_artifacts_0.1.2/another-subdirectory/' end end @@ -48,7 +49,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do it 'matches correct paths' do expect(subject.first). - to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', + to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/', + 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' end end -- cgit v1.2.1 From cd3b8bbd2f8e7ad75a453441f83c46aeb1d37353 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 14:18:06 +0100 Subject: Add method that checks if path exists in `StringPath` --- spec/lib/gitlab/string_path_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index a54bf109c80..861eb951236 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -32,6 +32,7 @@ describe Gitlab::StringPath do it { is_expected.to be_file } it { is_expected.to have_parent } it { is_expected.to_not have_descendants } + it { is_expected.to exist } describe '#basename' do subject { |example| path(example).basename } @@ -170,4 +171,16 @@ describe Gitlab::StringPath do it { is_expected.to eq '/path/file1' } end + + describe '#exists?', path: 'another_file' do + subject { |example| path(example).exists? } + it { is_expected.to be true } + end + + describe '#exists?', path: './non_existent/' do + let(:universe) { ['./something'] } + subject { |example| path(example).exists? } + + it { is_expected.to be false } + end end -- cgit v1.2.1 From 1b1793c2530d7003d8baa5aa1912a4ab258d4a1c Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 4 Jan 2016 15:07:49 +0100 Subject: Show file size in artifacts browser using metadata --- spec/lib/gitlab/string_path_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb index 861eb951236..7f1d111478b 100644 --- a/spec/lib/gitlab/string_path_spec.rb +++ b/spec/lib/gitlab/string_path_spec.rb @@ -162,7 +162,7 @@ describe Gitlab::StringPath do end let(:metadata) do - [{ name: '/path/'}, { name: '/path/file1' }, { name: '/path/file2' }] + [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] end subject do -- cgit v1.2.1 From 387b27813d1d496c015f4f174812b4761c32648d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 8 Jan 2016 12:35:49 +0100 Subject: Change format of artifacts metadata from text to binary 0.0.1 This changes the format of metadata to handle paths, that may contain whitespace characters, new line characters and non-UTF-8 characters. Now those paths along with metadata in JSON format are stored as length-prefixed strings (uint32 prefix). Metadata file has a custom format: 1. First string field is metadata version field (string) 2. Second string field is metadata errors field (JSON strong) 3. All subsequent fields is pair of path (string) and path metadata in JSON format. Path's metadata contains all fields that where possible to extract from ZIP archive like date of modification, CRC, compressed size, uncompressed size and comment. --- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 62c86a60ac4..0c8a41cfab7 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -59,6 +59,21 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do subject { metadata('').to_string_path } it { is_expected.to be_an_instance_of(Gitlab::StringPath) } end + + describe '#full_version' do + subject { metadata('').full_version } + it { is_expected.to eq 'GitLab Build Artifacts Metadata 0.0.1' } + end + + describe '#version' do + subject { metadata('').version } + it { is_expected.to eq '0.0.1' } + end + + describe '#errors' do + subject { metadata('').errors } + it { is_expected.to eq({}) } + end end context 'metadata file does not exist' do -- cgit v1.2.1 From 61fb47a43202332fe9ac57847996da929ba42d3f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sat, 9 Jan 2016 14:41:43 +0100 Subject: Simplify implementation of build artifacts browser (refactoring) --- .../ci/build/artifacts/metadata/path_spec.rb | 148 ++++++++++++++++ .../lib/gitlab/ci/build/artifacts/metadata_spec.rb | 22 +-- spec/lib/gitlab/string_path_spec.rb | 186 --------------------- 3 files changed, 154 insertions(+), 202 deletions(-) create mode 100644 spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb delete mode 100644 spec/lib/gitlab/string_path_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb new file mode 100644 index 00000000000..148d05b5902 --- /dev/null +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb @@ -0,0 +1,148 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Artifacts::Metadata::Path do + let(:universe) do + ['path/', + 'path/dir_1/', + 'path/dir_1/file_1', + 'path/dir_1/file_b', + 'path/dir_1/subdir/', + 'path/dir_1/subdir/subfile', + 'path/second_dir', + 'path/second_dir/dir_3/file_2', + 'path/second_dir/dir_3/file_3', + 'another_directory/', + 'another_file', + '/file/with/absolute_path'] + end + + def path(example) + string_path(example.metadata[:path]) + end + + def string_path(string_path) + described_class.new(string_path, universe) + end + + describe '/file/with/absolute_path', path: '/file/with/absolute_path' do + subject { |example| path(example) } + + it { is_expected.to be_file } + it { is_expected.to have_parent } + + describe '#basename' do + subject { |example| path(example).basename } + it { is_expected.to eq 'absolute_path' } + end + end + + describe 'path/dir_1/', path: 'path/dir_1/' do + subject { |example| path(example) } + it { is_expected.to have_parent } + it { is_expected.to be_directory } + + describe '#basename' do + subject { |example| path(example).basename } + it { is_expected.to eq 'dir_1/' } + end + + describe '#name' do + subject { |example| path(example).name } + it { is_expected.to eq 'dir_1' } + end + + describe '#parent' do + subject { |example| path(example).parent } + it { is_expected.to eq string_path('path/') } + end + + describe '#children' do + subject { |example| path(example).children } + + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/') + end + end + + describe '#files' do + subject { |example| path(example).files } + + it { is_expected.to all(be_file) } + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b') + end + end + + describe '#directories' do + subject { |example| path(example).directories } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } + end + + describe '#directories!' do + subject { |example| path(example).directories! } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/subdir/'), + string_path('path/') + end + end + end + + describe 'empty path', path: '' do + subject { |example| path(example) } + it { is_expected.to_not have_parent } + + describe '#children' do + subject { |example| path(example).children } + it { expect(subject.count).to eq 3 } + end + end + + describe '#nodes', path: './test' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + + describe '#nodes', path: './test/' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + + describe '#metadata' do + let(:universe) do + ['path/', 'path/file1', 'path/file2'] + end + + let(:metadata) do + [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] + end + + subject do + described_class.new('path/file1', universe, metadata).metadata[:name] + end + + it { is_expected.to eq '/path/file1' } + end + + describe '#exists?', path: 'another_file' do + subject { |example| path(example).exists? } + it { is_expected.to be true } + end + + describe '#exists?', path: './non_existent/' do + let(:universe) { ['./something'] } + subject { |example| path(example).exists? } + + it { is_expected.to be false } + end +end diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 0c8a41cfab7..36c4851126c 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -10,13 +10,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end context 'metadata file exists' do - describe '#exists?' do - subject { metadata.exists? } - it { is_expected.to be true } - end - - describe '#match! ./' do - subject { metadata('./').match! } + describe '#match! empty string' do + subject { metadata('').match! } it 'matches correct paths' do expect(subject.first).to contain_exactly 'ci_artifacts.txt', @@ -55,9 +50,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#to_string_path' do - subject { metadata('').to_string_path } - it { is_expected.to be_an_instance_of(Gitlab::StringPath) } + describe '#to_path' do + subject { metadata('').to_path } + it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metdata::Path) } end describe '#full_version' do @@ -79,14 +74,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do context 'metadata file does not exist' do let(:metadata_file_path) { '' } - describe '#exists?' do - subject { metadata.exists? } - it { is_expected.to be false } - end - describe '#match!' do it 'raises error' do - expect { metadata.match! }.to raise_error(StandardError, /Metadata file not found/) + expect { metadata.match! }.to raise_error(Errno::ENOENT) end end end diff --git a/spec/lib/gitlab/string_path_spec.rb b/spec/lib/gitlab/string_path_spec.rb deleted file mode 100644 index 7f1d111478b..00000000000 --- a/spec/lib/gitlab/string_path_spec.rb +++ /dev/null @@ -1,186 +0,0 @@ -require 'spec_helper' - -describe Gitlab::StringPath do - let(:universe) do - ['path/', - 'path/dir_1/', - 'path/dir_1/file_1', - 'path/dir_1/file_b', - 'path/dir_1/subdir/', - 'path/dir_1/subdir/subfile', - 'path/second_dir', - 'path/second_dir/dir_3/file_2', - 'path/second_dir/dir_3/file_3', - 'another_directory/', - 'another_file', - '/file/with/absolute_path'] - end - - def path(example) - string_path(example.metadata[:path]) - end - - def string_path(string_path) - described_class.new(string_path, universe) - end - - describe '/file/with/absolute_path', path: '/file/with/absolute_path' do - subject { |example| path(example) } - - it { is_expected.to be_absolute } - it { is_expected.to_not be_relative } - it { is_expected.to be_file } - it { is_expected.to have_parent } - it { is_expected.to_not have_descendants } - it { is_expected.to exist } - - describe '#basename' do - subject { |example| path(example).basename } - - it { is_expected.to eq 'absolute_path' } - end - end - - describe 'path/', path: 'path/' do - subject { |example| path(example) } - - it { is_expected.to be_directory } - it { is_expected.to be_relative } - end - - describe 'path/dir_1/', path: 'path/dir_1/' do - subject { |example| path(example) } - it { is_expected.to have_parent } - - describe '#basename' do - subject { |example| path(example).basename } - it { is_expected.to eq 'dir_1/' } - end - - describe '#name' do - subject { |example| path(example).name } - it { is_expected.to eq 'dir_1' } - end - - describe '#parent' do - subject { |example| path(example).parent } - it { is_expected.to eq string_path('path/') } - end - - describe '#descendants' do - subject { |example| path(example).descendants } - - it { is_expected.to be_an_instance_of Array } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/'), - string_path('path/dir_1/subdir/subfile') - end - end - - describe '#children' do - subject { |example| path(example).children } - - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/') - end - end - - describe '#files' do - subject { |example| path(example).files } - - it { is_expected.to all(be_file) } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b') - end - end - - describe '#directories' do - subject { |example| path(example).directories } - - it { is_expected.to all(be_directory) } - it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } - end - - describe '#directories!' do - subject { |example| path(example).directories! } - - it { is_expected.to all(be_directory) } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/dir_1/../') - end - end - end - - describe './', path: './' do - subject { |example| path(example) } - - it { is_expected.to_not have_parent } - it { is_expected.to have_descendants } - - describe '#descendants' do - subject { |example| path(example).descendants } - - it { expect(subject.count).to eq universe.count - 1 } - it { is_expected.to_not include string_path('./') } - end - - describe '#children' do - subject { |example| path(example).children } - it { expect(subject.count).to eq 3 } - end - end - - describe '#nodes', path: './' do - subject { |example| path(example).nodes } - it { is_expected.to eq 1 } - end - - describe '#nodes', path: './test' do - subject { |example| path(example).nodes } - it { is_expected.to eq 2 } - end - - describe '#nodes', path: './test/' do - subject { |example| path(example).nodes } - it { is_expected.to eq 2 } - end - - describe '#metadata' do - let(:universe) do - ['path/', 'path/file1', 'path/file2'] - end - - let(:metadata) do - [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] - end - - subject do - described_class.new('path/file1', universe, metadata).metadata[:name] - end - - it { is_expected.to eq '/path/file1' } - end - - describe '#exists?', path: 'another_file' do - subject { |example| path(example).exists? } - it { is_expected.to be true } - end - - describe '#exists?', path: './non_existent/' do - let(:universe) { ['./something'] } - subject { |example| path(example).exists? } - - it { is_expected.to be false } - end -end -- cgit v1.2.1 From 09a4a5aff8c53dd5930044ddbb285a95ef177d8a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 11 Jan 2016 09:57:03 +0100 Subject: Render only valid paths in artifacts metadata In this version we will support only relative paths in artifacts metadata. Support for absolute paths will be introduced later. --- spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb | 8 ++++---- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb index 148d05b5902..3b254c3ce2f 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb @@ -108,14 +108,14 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Path do end end - describe '#nodes', path: './test' do + describe '#nodes', path: 'test' do subject { |example| path(example).nodes } - it { is_expected.to eq 2 } + it { is_expected.to eq 1 } end - describe '#nodes', path: './test/' do + describe '#nodes', path: 'test/' do subject { |example| path(example).nodes } - it { is_expected.to eq 2 } + it { is_expected.to eq 1 } end describe '#metadata' do diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 36c4851126c..456314768be 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -28,8 +28,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2' do - subject { metadata('other_artifacts_0.1.2').match! } + describe '#match! other_artifacts_0.1.2/' do + subject { metadata('other_artifacts_0.1.2/').match! } it 'matches correct paths' do expect(subject.first). @@ -39,7 +39,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2/another-subdirectory' do + describe '#match! other_artifacts_0.1.2/another-subdirectory/' do subject { metadata('other_artifacts_0.1.2/another-subdirectory/').match! } it 'matches correct paths' do @@ -52,7 +52,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do describe '#to_path' do subject { metadata('').to_path } - it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metdata::Path) } + it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Path) } end describe '#full_version' do -- cgit v1.2.1 From cf00a808cc9896093be209dc5d6bfbea93b6226b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 12 Jan 2016 11:50:36 +0100 Subject: Fix specs for artifacts metadata after changing fixture content --- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 456314768be..5c1a94974e8 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -16,11 +16,12 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do it 'matches correct paths' do expect(subject.first).to contain_exactly 'ci_artifacts.txt', 'other_artifacts_0.1.2/', - 'rails_sample.jpg' + 'rails_sample.jpg', + 'tests_encoding/' end it 'matches metadata for every path' do - expect(subject.last.count).to eq 3 + expect(subject.last.count).to eq 4 end it 'return Hashes for each metadata' do -- cgit v1.2.1 From 6b0a43aff36f0bbb9050b3c04155a3ccd9c1a75b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 21:17:28 +0100 Subject: Improve readability of artifacts browser `Entry` related code --- .../ci/build/artifacts/metadata/entry_spec.rb | 170 +++++++++++++++++++++ .../ci/build/artifacts/metadata/path_spec.rb | 148 ------------------ .../lib/gitlab/ci/build/artifacts/metadata_spec.rb | 6 +- 3 files changed, 173 insertions(+), 151 deletions(-) create mode 100644 spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb delete mode 100644 spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb new file mode 100644 index 00000000000..a728227f87c --- /dev/null +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb @@ -0,0 +1,170 @@ +require 'spec_helper' + +describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do + let(:entries) do + ['path/', + 'path/dir_1/', + 'path/dir_1/file_1', + 'path/dir_1/file_b', + 'path/dir_1/subdir/', + 'path/dir_1/subdir/subfile', + 'path/second_dir', + 'path/second_dir/dir_3/file_2', + 'path/second_dir/dir_3/file_3', + 'another_directory/', + 'another_file', + '/file/with/absolute_path'] + end + + def path(example) + string_path(example.metadata[:path]) + end + + def string_path(string_path) + described_class.new(string_path, entries) + end + + describe '/file/with/absolute_path', path: '/file/with/absolute_path' do + subject { |example| path(example) } + + it { is_expected.to be_file } + it { is_expected.to have_parent } + + describe '#basename' do + subject { |example| path(example).basename } + it { is_expected.to eq 'absolute_path' } + end + end + + describe 'path/dir_1/', path: 'path/dir_1/' do + subject { |example| path(example) } + it { is_expected.to have_parent } + it { is_expected.to be_directory } + + describe '#basename' do + subject { |example| path(example).basename } + it { is_expected.to eq 'dir_1/' } + end + + describe '#name' do + subject { |example| path(example).name } + it { is_expected.to eq 'dir_1' } + end + + describe '#parent' do + subject { |example| path(example).parent } + it { is_expected.to eq string_path('path/') } + end + + describe '#children' do + subject { |example| path(example).children } + + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b'), + string_path('path/dir_1/subdir/') + end + end + + describe '#files' do + subject { |example| path(example).files } + + it { is_expected.to all(be_file) } + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/file_1'), + string_path('path/dir_1/file_b') + end + end + + describe '#directories' do + context 'without options' do + subject { |example| path(example).directories } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } + end + + context 'with option parent: true' do + subject { |example| path(example).directories(parent: true) } + + it { is_expected.to all(be_directory) } + it { is_expected.to all(be_an_instance_of described_class) } + it do + is_expected.to contain_exactly string_path('path/dir_1/subdir/'), + string_path('path/') + end + end + + describe '#nodes' do + subject { |example| path(example).nodes } + it { is_expected.to eq 2 } + end + + describe '#exists?' do + subject { |example| path(example).exists? } + it { is_expected.to be true } + end + + describe '#empty?' do + subject { |example| path(example).empty? } + it { is_expected.to be false } + end + end + end + + describe 'empty path', path: '' do + subject { |example| path(example) } + it { is_expected.to_not have_parent } + + describe '#children' do + subject { |example| path(example).children } + it { expect(subject.count).to eq 3 } + end + + end + + describe 'path/dir_1/subdir/subfile', path: 'path/dir_1/subdir/subfile' do + describe '#nodes' do + subject { |example| path(example).nodes } + it { is_expected.to eq 4 } + end + end + + describe 'non-existent/', path: 'non-existent/' do + describe '#empty?' do + subject { |example| path(example).empty? } + it { is_expected.to be true } + end + + describe '#exists?' do + subject { |example| path(example).exists? } + it { is_expected.to be false } + end + end + + describe 'another_directory/', path: 'another_directory/' do + describe '#empty?' do + subject { |example| path(example).empty? } + it { is_expected.to be true } + end + end + + describe '#metadata' do + let(:entries) do + ['path/', 'path/file1', 'path/file2'] + end + + let(:metadata) do + [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] + end + + subject do + described_class.new('path/file1', entries, metadata).metadata[:name] + end + + it { is_expected.to eq '/path/file1' } + end +end diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb deleted file mode 100644 index 3b254c3ce2f..00000000000 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/path_spec.rb +++ /dev/null @@ -1,148 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Ci::Build::Artifacts::Metadata::Path do - let(:universe) do - ['path/', - 'path/dir_1/', - 'path/dir_1/file_1', - 'path/dir_1/file_b', - 'path/dir_1/subdir/', - 'path/dir_1/subdir/subfile', - 'path/second_dir', - 'path/second_dir/dir_3/file_2', - 'path/second_dir/dir_3/file_3', - 'another_directory/', - 'another_file', - '/file/with/absolute_path'] - end - - def path(example) - string_path(example.metadata[:path]) - end - - def string_path(string_path) - described_class.new(string_path, universe) - end - - describe '/file/with/absolute_path', path: '/file/with/absolute_path' do - subject { |example| path(example) } - - it { is_expected.to be_file } - it { is_expected.to have_parent } - - describe '#basename' do - subject { |example| path(example).basename } - it { is_expected.to eq 'absolute_path' } - end - end - - describe 'path/dir_1/', path: 'path/dir_1/' do - subject { |example| path(example) } - it { is_expected.to have_parent } - it { is_expected.to be_directory } - - describe '#basename' do - subject { |example| path(example).basename } - it { is_expected.to eq 'dir_1/' } - end - - describe '#name' do - subject { |example| path(example).name } - it { is_expected.to eq 'dir_1' } - end - - describe '#parent' do - subject { |example| path(example).parent } - it { is_expected.to eq string_path('path/') } - end - - describe '#children' do - subject { |example| path(example).children } - - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/') - end - end - - describe '#files' do - subject { |example| path(example).files } - - it { is_expected.to all(be_file) } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b') - end - end - - describe '#directories' do - subject { |example| path(example).directories } - - it { is_expected.to all(be_directory) } - it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } - end - - describe '#directories!' do - subject { |example| path(example).directories! } - - it { is_expected.to all(be_directory) } - it { is_expected.to all(be_an_instance_of described_class) } - it do - is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/') - end - end - end - - describe 'empty path', path: '' do - subject { |example| path(example) } - it { is_expected.to_not have_parent } - - describe '#children' do - subject { |example| path(example).children } - it { expect(subject.count).to eq 3 } - end - end - - describe '#nodes', path: 'test' do - subject { |example| path(example).nodes } - it { is_expected.to eq 1 } - end - - describe '#nodes', path: 'test/' do - subject { |example| path(example).nodes } - it { is_expected.to eq 1 } - end - - describe '#metadata' do - let(:universe) do - ['path/', 'path/file1', 'path/file2'] - end - - let(:metadata) do - [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] - end - - subject do - described_class.new('path/file1', universe, metadata).metadata[:name] - end - - it { is_expected.to eq '/path/file1' } - end - - describe '#exists?', path: 'another_file' do - subject { |example| path(example).exists? } - it { is_expected.to be true } - end - - describe '#exists?', path: './non_existent/' do - let(:universe) { ['./something'] } - subject { |example| path(example).exists? } - - it { is_expected.to be false } - end -end diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 5c1a94974e8..42fbe40c890 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -51,9 +51,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#to_path' do - subject { metadata('').to_path } - it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Path) } + describe '#to_entry' do + subject { metadata('').to_entry } + it { is_expected.to be_an_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) } end describe '#full_version' do -- cgit v1.2.1 From ad2b0358e0facd5c65c4141ce54c2e55bab165e6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 22:31:27 +0100 Subject: Improve readability of artifacts `Metadata` related code --- spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 42fbe40c890..8560493f5b5 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -10,8 +10,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end context 'metadata file exists' do - describe '#match! empty string' do - subject { metadata('').match! } + describe '#find_entries! empty string' do + subject { metadata('').find_entries! } it 'matches correct paths' do expect(subject.first).to contain_exactly 'ci_artifacts.txt', @@ -29,8 +29,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2/' do - subject { metadata('other_artifacts_0.1.2/').match! } + describe '#find_entries! other_artifacts_0.1.2/' do + subject { metadata('other_artifacts_0.1.2/').find_entries! } it 'matches correct paths' do expect(subject.first). @@ -40,8 +40,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do end end - describe '#match! other_artifacts_0.1.2/another-subdirectory/' do - subject { metadata('other_artifacts_0.1.2/another-subdirectory/').match! } + describe '#find_entries! other_artifacts_0.1.2/another-subdirectory/' do + subject { metadata('other_artifacts_0.1.2/another-subdirectory/').find_entries! } it 'matches correct paths' do expect(subject.first). @@ -75,9 +75,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do context 'metadata file does not exist' do let(:metadata_file_path) { '' } - describe '#match!' do + describe '#find_entries!' do it 'raises error' do - expect { metadata.match! }.to raise_error(Errno::ENOENT) + expect { metadata.find_entries! }.to raise_error(Errno::ENOENT) end end end -- cgit v1.2.1 From 0d6e7b9d3d38e60e5a706956a853e7dc940e4574 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 13 Jan 2016 23:24:28 +0100 Subject: Use Hash to store paths and entries metadata in artifacts browser --- .../ci/build/artifacts/metadata/entry_spec.rb | 58 +++++++++++----------- .../lib/gitlab/ci/build/artifacts/metadata_spec.rb | 16 +++--- 2 files changed, 36 insertions(+), 38 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb index a728227f87c..41257103ead 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata/entry_spec.rb @@ -2,26 +2,26 @@ require 'spec_helper' describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do let(:entries) do - ['path/', - 'path/dir_1/', - 'path/dir_1/file_1', - 'path/dir_1/file_b', - 'path/dir_1/subdir/', - 'path/dir_1/subdir/subfile', - 'path/second_dir', - 'path/second_dir/dir_3/file_2', - 'path/second_dir/dir_3/file_3', - 'another_directory/', - 'another_file', - '/file/with/absolute_path'] + { 'path/' => {}, + 'path/dir_1/' => {}, + 'path/dir_1/file_1' => {}, + 'path/dir_1/file_b' => {}, + 'path/dir_1/subdir/' => {}, + 'path/dir_1/subdir/subfile' => {}, + 'path/second_dir' => {}, + 'path/second_dir/dir_3/file_2' => {}, + 'path/second_dir/dir_3/file_3'=> {}, + 'another_directory/'=> {}, + 'another_file' => {}, + '/file/with/absolute_path' => {} } end def path(example) - string_path(example.metadata[:path]) + entry(example.metadata[:path]) end - def string_path(string_path) - described_class.new(string_path, entries) + def entry(path) + described_class.new(path, entries) end describe '/file/with/absolute_path', path: '/file/with/absolute_path' do @@ -53,7 +53,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do describe '#parent' do subject { |example| path(example).parent } - it { is_expected.to eq string_path('path/') } + it { is_expected.to eq entry('path/') } end describe '#children' do @@ -61,9 +61,9 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do it { is_expected.to all(be_an_instance_of described_class) } it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b'), - string_path('path/dir_1/subdir/') + is_expected.to contain_exactly entry('path/dir_1/file_1'), + entry('path/dir_1/file_b'), + entry('path/dir_1/subdir/') end end @@ -73,8 +73,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do it { is_expected.to all(be_file) } it { is_expected.to all(be_an_instance_of described_class) } it do - is_expected.to contain_exactly string_path('path/dir_1/file_1'), - string_path('path/dir_1/file_b') + is_expected.to contain_exactly entry('path/dir_1/file_1'), + entry('path/dir_1/file_b') end end @@ -84,7 +84,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do it { is_expected.to all(be_directory) } it { is_expected.to all(be_an_instance_of described_class) } - it { is_expected.to contain_exactly string_path('path/dir_1/subdir/') } + it { is_expected.to contain_exactly entry('path/dir_1/subdir/') } end context 'with option parent: true' do @@ -93,8 +93,8 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do it { is_expected.to all(be_directory) } it { is_expected.to all(be_an_instance_of described_class) } it do - is_expected.to contain_exactly string_path('path/dir_1/subdir/'), - string_path('path/') + is_expected.to contain_exactly entry('path/dir_1/subdir/'), + entry('path/') end end @@ -154,15 +154,13 @@ describe Gitlab::Ci::Build::Artifacts::Metadata::Entry do describe '#metadata' do let(:entries) do - ['path/', 'path/file1', 'path/file2'] - end - - let(:metadata) do - [{ name: '/path/' }, { name: '/path/file1' }, { name: '/path/file2' }] + { 'path/' => { name: '/path/' }, + 'path/file1' => { name: '/path/file1' }, + 'path/file2' => { name: '/path/file2' } } end subject do - described_class.new('path/file1', entries, metadata).metadata[:name] + described_class.new('path/file1', entries).metadata[:name] end it { is_expected.to eq '/path/file1' } diff --git a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb index 8560493f5b5..828eedfa7b0 100644 --- a/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb +++ b/spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb @@ -14,18 +14,18 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do subject { metadata('').find_entries! } it 'matches correct paths' do - expect(subject.first).to contain_exactly 'ci_artifacts.txt', - 'other_artifacts_0.1.2/', - 'rails_sample.jpg', - 'tests_encoding/' + expect(subject.keys).to contain_exactly 'ci_artifacts.txt', + 'other_artifacts_0.1.2/', + 'rails_sample.jpg', + 'tests_encoding/' end it 'matches metadata for every path' do - expect(subject.last.count).to eq 4 + expect(subject.keys.count).to eq 4 end it 'return Hashes for each metadata' do - expect(subject.last).to all(be_kind_of(Hash)) + expect(subject.values).to all(be_kind_of(Hash)) end end @@ -33,7 +33,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do subject { metadata('other_artifacts_0.1.2/').find_entries! } it 'matches correct paths' do - expect(subject.first). + expect(subject.keys). to contain_exactly 'other_artifacts_0.1.2/', 'other_artifacts_0.1.2/doc_sample.txt', 'other_artifacts_0.1.2/another-subdirectory/' @@ -44,7 +44,7 @@ describe Gitlab::Ci::Build::Artifacts::Metadata do subject { metadata('other_artifacts_0.1.2/another-subdirectory/').find_entries! } it 'matches correct paths' do - expect(subject.first). + expect(subject.keys). to contain_exactly 'other_artifacts_0.1.2/another-subdirectory/', 'other_artifacts_0.1.2/another-subdirectory/empty_directory/', 'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' -- cgit v1.2.1 From 78f5eb94fb15f7e4cc4208a4871b9533243bec40 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Wed, 6 Jan 2016 21:15:37 -0200 Subject: Import GitHub wiki into GitLab --- .../gitlab/github_import/wiki_formatter_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 spec/lib/gitlab/github_import/wiki_formatter_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb new file mode 100644 index 00000000000..a4ef7b60ae1 --- /dev/null +++ b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::WikiFormatter, lib: true do + let(:project) do + create(:project, namespace: create(:namespace, path: 'gitlabhq'), + import_url: 'https://xxx@github.com/gitlabhq/gitlabhq.git') + end + + subject(:wiki) { described_class.new(project)} + + describe '#path_with_namespace' do + it 'appends .wiki to project path' do + expect(wiki.path_with_namespace).to eq 'gitlabhq/gitlabhq.wiki' + end + end + + describe '#import_url' do + it 'returns URL of the wiki repository' do + expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/gitlabhq.wiki.git' + end + end +end -- cgit v1.2.1 From a6a5990ee5f504107944c3bba5c18dbdea9f5207 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Jan 2016 02:05:18 -0200 Subject: Add Banzai::Filter::GollumTagsFilter for parsing Gollum's tags in HTML --- spec/lib/banzai/filter/gollum_tags_filter_spec.rb | 93 +++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 spec/lib/banzai/filter/gollum_tags_filter_spec.rb (limited to 'spec/lib') diff --git a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb new file mode 100644 index 00000000000..530b37526b3 --- /dev/null +++ b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb @@ -0,0 +1,93 @@ +require 'spec_helper' + +describe Banzai::Filter::GollumTagsFilter, lib: true do + include FilterSpecHelper + + let(:project) { create(:project) } + let(:user) { double } + let(:project_wiki) { ProjectWiki.new(project, user) } + + describe 'validation' do + it 'ensure that a :project_wiki key exists in context' do + expect { filter("See [[images/image.jpg]]", {}) }.to raise_error ArgumentError, "Missing context keys for Banzai::Filter::GollumTagsFilter: :project_wiki" + end + end + + context 'linking internal images' do + it 'creates img tag if image exists' do + file = Gollum::File.new(project_wiki.wiki) + expect(file).to receive(:path).and_return('images/image.jpg') + expect(project_wiki).to receive(:find_file).with('images/image.jpg').and_return(file) + + tag = '[[images/image.jpg]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('img')['src']).to eq "#{project_wiki.wiki_base_path}/images/image.jpg" + end + + it 'does not creates img tag if image does not exist' do + expect(project_wiki).to receive(:find_file).with('images/image.jpg').and_return(nil) + + tag = '[[images/image.jpg]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.css('img').size).to eq 0 + end + end + + context 'linking external images' do + it 'creates img tag for valid URL' do + expect(project_wiki).to receive(:find_file).with('http://example.com/image.jpg').and_return(nil) + + tag = '[[http://example.com/image.jpg]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('img')['src']).to eq "http://example.com/image.jpg" + end + + it 'does not creates img tag for invalid URL' do + expect(project_wiki).to receive(:find_file).with('http://example.com/image.pdf').and_return(nil) + + tag = '[[http://example.com/image.pdf]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.css('img').size).to eq 0 + end + end + + context 'linking external resources' do + it "the created link's text will be equal to the resource's text" do + tag = '[[http://example.com]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('a').text).to eq 'http://example.com' + expect(doc.at_css('a')['href']).to eq 'http://example.com' + end + + it "the created link's text will be link-text" do + tag = '[[link-text|http://example.com/pdfs/gollum.pdf]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('a').text).to eq 'link-text' + expect(doc.at_css('a')['href']).to eq 'http://example.com/pdfs/gollum.pdf' + end + end + + context 'linking internal resources' do + it "the created link's text will be equal to the resource's text" do + tag = '[[wiki-slug]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('a').text).to eq 'wiki-slug' + expect(doc.at_css('a')['href']).to eq 'wiki-slug' + end + + it "the created link's text will be link-text" do + tag = '[[link-text|wiki-slug]]' + doc = filter("See #{tag}", project_wiki: project_wiki) + + expect(doc.at_css('a').text).to eq 'link-text' + expect(doc.at_css('a')['href']).to eq 'wiki-slug' + end + end +end -- cgit v1.2.1 From 89e8b82b638e440bc487c4135cdfa4402fdffde5 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Jan 2016 14:50:25 -0200 Subject: Make sure the .git is at the end on Gitlab::GithubImport::WikiFormatter --- spec/lib/gitlab/github_import/wiki_formatter_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb index a4ef7b60ae1..aed2aa39e3a 100644 --- a/spec/lib/gitlab/github_import/wiki_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/wiki_formatter_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Gitlab::GithubImport::WikiFormatter, lib: true do let(:project) do create(:project, namespace: create(:namespace, path: 'gitlabhq'), - import_url: 'https://xxx@github.com/gitlabhq/gitlabhq.git') + import_url: 'https://xxx@github.com/gitlabhq/sample.gitlabhq.git') end subject(:wiki) { described_class.new(project)} @@ -16,7 +16,7 @@ describe Gitlab::GithubImport::WikiFormatter, lib: true do describe '#import_url' do it 'returns URL of the wiki repository' do - expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/gitlabhq.wiki.git' + expect(wiki.import_url).to eq 'https://xxx@github.com/gitlabhq/sample.gitlabhq.wiki.git' end end end -- cgit v1.2.1 From 765a2c73271cf311311c391e7e64f83e141c79ae Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 12 Jan 2016 18:59:49 -0200 Subject: Refactoring Banzai::Filter::GollumTagsFilter --- spec/lib/banzai/filter/gollum_tags_filter_spec.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'spec/lib') diff --git a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb index 530b37526b3..38baa819957 100644 --- a/spec/lib/banzai/filter/gollum_tags_filter_spec.rb +++ b/spec/lib/banzai/filter/gollum_tags_filter_spec.rb @@ -37,8 +37,6 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do context 'linking external images' do it 'creates img tag for valid URL' do - expect(project_wiki).to receive(:find_file).with('http://example.com/image.jpg').and_return(nil) - tag = '[[http://example.com/image.jpg]]' doc = filter("See #{tag}", project_wiki: project_wiki) @@ -46,8 +44,6 @@ describe Banzai::Filter::GollumTagsFilter, lib: true do end it 'does not creates img tag for invalid URL' do - expect(project_wiki).to receive(:find_file).with('http://example.com/image.pdf').and_return(nil) - tag = '[[http://example.com/image.pdf]]' doc = filter("See #{tag}", project_wiki: project_wiki) -- cgit v1.2.1