From ea4777ff501e370a39ae30e76a955136afe3c1fa Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 15:19:13 +0100
Subject: Add features for list and show details of variables in API

---
 spec/factories/ci/variables.rb      | 25 +++++++++++++
 spec/requests/api/variables_spec.rb | 75 +++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+)
 create mode 100644 spec/factories/ci/variables.rb
 create mode 100644 spec/requests/api/variables_spec.rb

(limited to 'spec')

diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb
new file mode 100644
index 00000000000..c3dcb678da7
--- /dev/null
+++ b/spec/factories/ci/variables.rb
@@ -0,0 +1,25 @@
+# == Schema Information
+#
+# Table name: ci_variables
+#
+#  id                   :integer          not null, primary key
+#  project_id           :integer          not null
+#  key                  :string(255)
+#  value                :text
+#  encrypted_value      :text
+#  encrypted_value_salt :string(255)
+#  encrypted_value_iv   :string(255)
+#  gl_project_id        :integer
+#
+
+# Read about factories at https://github.com/thoughtbot/factory_girl
+
+FactoryGirl.define do
+  factory :ci_variable, class: Ci::Variable do
+    id 1
+    key 'TEST_VARIABLE_1'
+    value 'VALUE_1'
+
+    project factory: :empty_project
+  end
+end
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
new file mode 100644
index 00000000000..8f66f5432b6
--- /dev/null
+++ b/spec/requests/api/variables_spec.rb
@@ -0,0 +1,75 @@
+require 'spec_helper'
+
+describe API::API, api: true do
+  include ApiHelpers
+
+  let(:user) { create(:user) }
+  let(:user2) { create(:user) }
+  let!(:project) { create(:project, creator_id: user.id) }
+  let!(:master) { create(:project_member, user: user, project: project, access_level: ProjectMember::MASTER) }
+  let!(:developer) { create(:project_member, user: user2, project: project, access_level: ProjectMember::DEVELOPER) }
+  let!(:variable) { create(:ci_variable, project: project) }
+
+  describe 'GET /projects/:id/variables' do
+    context 'authorized user with proper permissions' do
+      it 'should return project variables' do
+        get api("/projects/#{project.id}/variables", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response).to be_a(Array)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not return project variables' do
+        get api("/projects/#{project.id}/variables", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not return project variables' do
+        get api("/projects/#{project.id}/variables")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe 'GET /projects/:id/variables/:variable_id' do
+    context 'authorized user with proper permissions' do
+      it 'should return project variable details when ID is used as :variable_id' do
+        get api("/projects/#{project.id}/variables/1", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response['key']).to eq('TEST_VARIABLE_1')
+        expect(json_response['value']).to eq('VALUE_1')
+      end
+
+      it 'should return project variable details when `key` is used as :variable_id' do
+        get api("/projects/#{project.id}/variables/TEST_VARIABLE_1", user)
+
+        expect(response.status).to eq(200)
+        expect(json_response['id']).to eq(1)
+        expect(json_response['value']).to eq('VALUE_1')
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not return project variable details' do
+        get api("/projects/#{project.id}/variables/1", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not return project variable details' do
+        get api("/projects/#{project.id}/variables/1")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+end
-- 
cgit v1.2.1


From a692ce1c079703c4f3947e1d0a29547189e94d0f Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 16:25:49 +0100
Subject: Add update feature for variables API

---
 spec/requests/api/variables_spec.rb | 52 +++++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 8 deletions(-)

(limited to 'spec')

diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 8f66f5432b6..3f58277c4ae 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -40,25 +40,25 @@ describe API::API, api: true do
   describe 'GET /projects/:id/variables/:variable_id' do
     context 'authorized user with proper permissions' do
       it 'should return project variable details when ID is used as :variable_id' do
-        get api("/projects/#{project.id}/variables/1", user)
+        get api("/projects/#{project.id}/variables/#{variable.id}", user)
 
         expect(response.status).to eq(200)
-        expect(json_response['key']).to eq('TEST_VARIABLE_1')
-        expect(json_response['value']).to eq('VALUE_1')
+        expect(json_response['key']).to eq(variable.key)
+        expect(json_response['value']).to eq(variable.value)
       end
 
       it 'should return project variable details when `key` is used as :variable_id' do
-        get api("/projects/#{project.id}/variables/TEST_VARIABLE_1", user)
+        get api("/projects/#{project.id}/variables/#{variable.key}", user)
 
         expect(response.status).to eq(200)
-        expect(json_response['id']).to eq(1)
-        expect(json_response['value']).to eq('VALUE_1')
+        expect(json_response['id']).to eq(variable.id)
+        expect(json_response['value']).to eq(variable.value)
       end
     end
 
     context 'authorized user with invalid permissions' do
       it 'should not return project variable details' do
-        get api("/projects/#{project.id}/variables/1", user2)
+        get api("/projects/#{project.id}/variables/#{variable.id}", user2)
 
         expect(response.status).to eq(403)
       end
@@ -66,7 +66,43 @@ describe API::API, api: true do
 
     context 'unauthorized user' do
       it 'should not return project variable details' do
-        get api("/projects/#{project.id}/variables/1")
+        get api("/projects/#{project.id}/variables/#{variable.id}")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
+  describe 'PUT /projects/:id/variables/:variable_id' do
+    context 'authorized user with proper permissions' do
+      it 'should update variable data' do
+        initial_variable = project.variables.first
+        key_before = initial_variable.key
+        value_before = initial_variable.value
+
+        put api("/projects/#{project.id}/variables/#{variable.id}", user), key: 'TEST_VARIABLE_1_UP', value: 'VALUE_1_UP'
+
+        updated_variable = project.variables.first
+
+        expect(response.status).to eq(200)
+        expect(key_before).to eq(variable.key)
+        expect(value_before).to eq(variable.value)
+        expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP')
+        expect(updated_variable.value).to eq('VALUE_1_UP')
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not update variable' do
+        put api("/projects/#{project.id}/variables/#{variable.id}", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not return project variable details' do
+        put api("/projects/#{project.id}/variables/#{variable.id}")
 
         expect(response.status).to eq(401)
       end
-- 
cgit v1.2.1


From 0d014feb1d216e692882976f0d70c3227eaec4ca Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 16:56:03 +0100
Subject: Add delete feature to variables API

---
 spec/requests/api/variables_spec.rb | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 3f58277c4ae..385db2409bd 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -101,11 +101,38 @@ describe API::API, api: true do
     end
 
     context 'unauthorized user' do
-      it 'should not return project variable details' do
+      it 'should not update variable' do
         put api("/projects/#{project.id}/variables/#{variable.id}")
 
         expect(response.status).to eq(401)
       end
     end
   end
+
+  describe 'DELETE /projects/:id/variables/:variable_id' do
+    context 'authorized user with proper permissions' do
+      it 'should delete variable' do
+        expect do
+          delete api("/projects/#{project.id}/variables/#{variable.id}", user)
+        end.to change{project.variables.count}.by(-1)
+        expect(response.status).to eq(200)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not delete variable' do
+        delete api("/projects/#{project.id}/variables/#{variable.id}", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not delete variable' do
+        delete api("/projects/#{project.id}/variables/#{variable.id}")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
 end
-- 
cgit v1.2.1


From c5177dd5e2171b047a695802c979cf779522ba8a Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 17:03:11 +0100
Subject: Add missing 'not_found' checks in variables API

---
 spec/requests/api/variables_spec.rb | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

(limited to 'spec')

diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 385db2409bd..b35ee2d32d1 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -54,6 +54,12 @@ describe API::API, api: true do
         expect(json_response['id']).to eq(variable.id)
         expect(json_response['value']).to eq(variable.value)
       end
+
+      it 'should responde with 404 Not Found if requesting non-existing variable' do
+        get api("/projects/#{project.id}/variables/9999", user)
+
+        expect(response.status).to eq(404)
+      end
     end
 
     context 'authorized user with invalid permissions' do
@@ -90,6 +96,12 @@ describe API::API, api: true do
         expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP')
         expect(updated_variable.value).to eq('VALUE_1_UP')
       end
+
+      it 'should responde with 404 Not Found if requesting non-existing variable' do
+        put api("/projects/#{project.id}/variables/9999", user)
+
+        expect(response.status).to eq(404)
+      end
     end
 
     context 'authorized user with invalid permissions' do
@@ -117,6 +129,12 @@ describe API::API, api: true do
         end.to change{project.variables.count}.by(-1)
         expect(response.status).to eq(200)
       end
+
+      it 'should responde with 404 Not Found if requesting non-existing variable' do
+        delete api("/projects/#{project.id}/variables/9999", user)
+
+        expect(response.status).to eq(404)
+      end
     end
 
     context 'authorized user with invalid permissions' do
-- 
cgit v1.2.1


From 937567b767e6d7b34dcaa1d9c83fc75464638683 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 31 Dec 2015 22:30:07 +0100
Subject: Add create feature to variables API

---
 spec/factories/ci/variables.rb      |  2 +-
 spec/requests/api/variables_spec.rb | 38 +++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb
index c3dcb678da7..a19b9fc72f2 100644
--- a/spec/factories/ci/variables.rb
+++ b/spec/factories/ci/variables.rb
@@ -16,7 +16,7 @@
 
 FactoryGirl.define do
   factory :ci_variable, class: Ci::Variable do
-    id 1
+    id 10
     key 'TEST_VARIABLE_1'
     value 'VALUE_1'
 
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index b35ee2d32d1..bf0dd77473a 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -79,6 +79,44 @@ describe API::API, api: true do
     end
   end
 
+  describe 'POST /projects/:id/variables' do
+    context 'authorized user with proper permissions' do
+      it 'should create variable' do
+        expect do
+          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2'
+        end.to change{project.variables.count}.by(1)
+
+        expect(response.status).to eq(201)
+        expect(json_response['key']).to eq('TEST_VARIABLE_2')
+        expect(json_response['value']).to eq('VALUE_2')
+      end
+
+      it 'should not allow to duplicate variable key' do
+        expect do
+          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_1', value: 'VALUE_2'
+        end.to change{project.variables.count}.by(0)
+
+        expect(response.status).to eq(400)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'should not create variable' do
+        post api("/projects/#{project.id}/variables", user2)
+
+        expect(response.status).to eq(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'should not create variable' do
+        post api("/projects/#{project.id}/variables")
+
+        expect(response.status).to eq(401)
+      end
+    end
+  end
+
   describe 'PUT /projects/:id/variables/:variable_id' do
     context 'authorized user with proper permissions' do
       it 'should update variable data' do
-- 
cgit v1.2.1


From b60c146267dfa8dc1c170426e1817c6b2a168d1a Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Thu, 7 Jan 2016 13:49:38 +0100
Subject: Change :variable_id to :key as resource ID in API

---
 spec/requests/api/variables_spec.rb | 42 +++++++++++++------------------------
 1 file changed, 15 insertions(+), 27 deletions(-)

(limited to 'spec')

diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index bf0dd77473a..214d7d5a0cc 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -37,26 +37,17 @@ describe API::API, api: true do
     end
   end
 
-  describe 'GET /projects/:id/variables/:variable_id' do
+  describe 'GET /projects/:id/variables/:key' do
     context 'authorized user with proper permissions' do
-      it 'should return project variable details when ID is used as :variable_id' do
-        get api("/projects/#{project.id}/variables/#{variable.id}", user)
-
-        expect(response.status).to eq(200)
-        expect(json_response['key']).to eq(variable.key)
-        expect(json_response['value']).to eq(variable.value)
-      end
-
-      it 'should return project variable details when `key` is used as :variable_id' do
+      it 'should return project variable details' do
         get api("/projects/#{project.id}/variables/#{variable.key}", user)
 
         expect(response.status).to eq(200)
-        expect(json_response['id']).to eq(variable.id)
         expect(json_response['value']).to eq(variable.value)
       end
 
       it 'should responde with 404 Not Found if requesting non-existing variable' do
-        get api("/projects/#{project.id}/variables/9999", user)
+        get api("/projects/#{project.id}/variables/non_existing_variable", user)
 
         expect(response.status).to eq(404)
       end
@@ -64,7 +55,7 @@ describe API::API, api: true do
 
     context 'authorized user with invalid permissions' do
       it 'should not return project variable details' do
-        get api("/projects/#{project.id}/variables/#{variable.id}", user2)
+        get api("/projects/#{project.id}/variables/#{variable.key}", user2)
 
         expect(response.status).to eq(403)
       end
@@ -72,7 +63,7 @@ describe API::API, api: true do
 
     context 'unauthorized user' do
       it 'should not return project variable details' do
-        get api("/projects/#{project.id}/variables/#{variable.id}")
+        get api("/projects/#{project.id}/variables/#{variable.key}")
 
         expect(response.status).to eq(401)
       end
@@ -117,26 +108,23 @@ describe API::API, api: true do
     end
   end
 
-  describe 'PUT /projects/:id/variables/:variable_id' do
+  describe 'PUT /projects/:id/variables/:key' do
     context 'authorized user with proper permissions' do
       it 'should update variable data' do
         initial_variable = project.variables.first
-        key_before = initial_variable.key
         value_before = initial_variable.value
 
-        put api("/projects/#{project.id}/variables/#{variable.id}", user), key: 'TEST_VARIABLE_1_UP', value: 'VALUE_1_UP'
+        put api("/projects/#{project.id}/variables/#{variable.key}", user), value: 'VALUE_1_UP'
 
         updated_variable = project.variables.first
 
         expect(response.status).to eq(200)
-        expect(key_before).to eq(variable.key)
         expect(value_before).to eq(variable.value)
-        expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP')
         expect(updated_variable.value).to eq('VALUE_1_UP')
       end
 
       it 'should responde with 404 Not Found if requesting non-existing variable' do
-        put api("/projects/#{project.id}/variables/9999", user)
+        put api("/projects/#{project.id}/variables/non_existing_variable", user)
 
         expect(response.status).to eq(404)
       end
@@ -144,7 +132,7 @@ describe API::API, api: true do
 
     context 'authorized user with invalid permissions' do
       it 'should not update variable' do
-        put api("/projects/#{project.id}/variables/#{variable.id}", user2)
+        put api("/projects/#{project.id}/variables/#{variable.key}", user2)
 
         expect(response.status).to eq(403)
       end
@@ -152,24 +140,24 @@ describe API::API, api: true do
 
     context 'unauthorized user' do
       it 'should not update variable' do
-        put api("/projects/#{project.id}/variables/#{variable.id}")
+        put api("/projects/#{project.id}/variables/#{variable.key}")
 
         expect(response.status).to eq(401)
       end
     end
   end
 
-  describe 'DELETE /projects/:id/variables/:variable_id' do
+  describe 'DELETE /projects/:id/variables/:key' do
     context 'authorized user with proper permissions' do
       it 'should delete variable' do
         expect do
-          delete api("/projects/#{project.id}/variables/#{variable.id}", user)
+          delete api("/projects/#{project.id}/variables/#{variable.key}", user)
         end.to change{project.variables.count}.by(-1)
         expect(response.status).to eq(200)
       end
 
       it 'should responde with 404 Not Found if requesting non-existing variable' do
-        delete api("/projects/#{project.id}/variables/9999", user)
+        delete api("/projects/#{project.id}/variables/non_existing_variable", user)
 
         expect(response.status).to eq(404)
       end
@@ -177,7 +165,7 @@ describe API::API, api: true do
 
     context 'authorized user with invalid permissions' do
       it 'should not delete variable' do
-        delete api("/projects/#{project.id}/variables/#{variable.id}", user2)
+        delete api("/projects/#{project.id}/variables/#{variable.key}", user2)
 
         expect(response.status).to eq(403)
       end
@@ -185,7 +173,7 @@ describe API::API, api: true do
 
     context 'unauthorized user' do
       it 'should not delete variable' do
-        delete api("/projects/#{project.id}/variables/#{variable.id}")
+        delete api("/projects/#{project.id}/variables/#{variable.key}")
 
         expect(response.status).to eq(401)
       end
-- 
cgit v1.2.1


From bc7ef8e5b7a002ca6bc2d7a5e6be11b4a59b6710 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 29 Dec 2015 17:53:55 -0200
Subject: Add ldap_blocked as new state to users state machine

---
 spec/models/user_spec.rb | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

(limited to 'spec')

diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 3cd63b2b0e8..0bef68e2885 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -569,27 +569,39 @@ describe User, models: true do
     end
   end
 
-  describe :ldap_user? do
-    it "is true if provider name starts with ldap" do
-      user = create(:omniauth_user, provider: 'ldapmain')
-      expect( user.ldap_user? ).to be_truthy
-    end
+  context 'ldap synchronized user' do
+    describe :ldap_user? do
+      it 'is true if provider name starts with ldap' do
+        user = create(:omniauth_user, provider: 'ldapmain')
+        expect(user.ldap_user?).to be_truthy
+      end
 
-    it "is false for other providers" do
-      user = create(:omniauth_user, provider: 'other-provider')
-      expect( user.ldap_user? ).to be_falsey
+      it 'is false for other providers' do
+        user = create(:omniauth_user, provider: 'other-provider')
+        expect(user.ldap_user?).to be_falsey
+      end
+
+      it 'is false if no extern_uid is provided' do
+        user = create(:omniauth_user, extern_uid: nil)
+        expect(user.ldap_user?).to be_falsey
+      end
     end
 
-    it "is false if no extern_uid is provided" do
-      user = create(:omniauth_user, extern_uid: nil)
-      expect( user.ldap_user? ).to be_falsey
+    describe :ldap_identity do
+      it 'returns ldap identity' do
+        user = create :omniauth_user
+        expect(user.ldap_identity.provider).not_to be_empty
+      end
     end
-  end
 
-  describe :ldap_identity do
-    it "returns ldap identity" do
-      user = create :omniauth_user
-      expect(user.ldap_identity.provider).not_to be_empty
+    describe '#ldap_block' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain', name: 'John Smith') }
+
+      it 'blocks user flaging the action caming from ldap' do
+        user.ldap_block
+        expect(user.blocked?).to be_truthy
+        expect(user.ldap_blocked?).to be_truthy
+      end
     end
   end
 
-- 
cgit v1.2.1


From ba9855d4877998e3574907cc542fcab15a9d1353 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 29 Dec 2015 18:58:38 -0200
Subject: Prevent ldap_blocked users from being unblocked by the Admin UI

---
 spec/controllers/admin/users_controller_spec.rb | 35 ++++++++++++++++++-------
 1 file changed, 26 insertions(+), 9 deletions(-)

(limited to 'spec')

diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb
index 8b7af4d3a0a..5b1f65d7aff 100644
--- a/spec/controllers/admin/users_controller_spec.rb
+++ b/spec/controllers/admin/users_controller_spec.rb
@@ -34,17 +34,34 @@ describe Admin::UsersController do
   end
 
   describe 'PUT unblock/:id' do
-    let(:user) { create(:user) }
-
-    before do
-      user.block
+    context 'ldap blocked users' do
+      let(:user) { create(:omniauth_user, provider: 'ldapmain') }
+
+      before do
+        user.ldap_block
+      end
+
+      it 'will not unblock user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_truthy
+        expect(flash[:alert]).to eq 'This user cannot be unlocked manually from GitLab'
+      end
     end
 
-    it 'unblocks user' do
-      put :unblock, id: user.username
-      user.reload
-      expect(user.blocked?).to be_falsey
-      expect(flash[:notice]).to eq 'Successfully unblocked'
+    context 'manually blocked users' do
+      let(:user) { create(:user) }
+
+      before do
+        user.block
+      end
+
+      it 'unblocks user' do
+        put :unblock, id: user.username
+        user.reload
+        expect(user.blocked?).to be_falsey
+        expect(flash[:notice]).to eq 'Successfully unblocked'
+      end
     end
   end
 
-- 
cgit v1.2.1


From 6e7db8e23e169bcbf0847ece27b9e44e00ae572b Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Wed, 30 Dec 2015 16:52:02 -0200
Subject: Prevent ldap_blocked users from being blocked/unblocked by the API

---
 spec/requests/api/users_spec.rb | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

(limited to 'spec')

diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb
index 4f278551d07..b82c5c7685f 100644
--- a/spec/requests/api/users_spec.rb
+++ b/spec/requests/api/users_spec.rb
@@ -8,6 +8,8 @@ describe API::API, api: true  do
   let(:key)   { create(:key, user: user) }
   let(:email)   { create(:email, user: user) }
   let(:omniauth_user) { create(:omniauth_user) }
+  let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') }
+  let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
 
   describe "GET /users" do
     context "when unauthenticated" do
@@ -783,6 +785,12 @@ describe API::API, api: true  do
       expect(user.reload.state).to eq('blocked')
     end
 
+    it 'should not re-block ldap blocked users' do
+      put api("/users/#{ldap_blocked_user.id}/block", admin)
+      expect(response.status).to eq(403)
+      expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
+    end
+
     it 'should not be available for non admin users' do
       put api("/users/#{user.id}/block", user)
       expect(response.status).to eq(403)
@@ -797,7 +805,9 @@ describe API::API, api: true  do
   end
 
   describe 'PUT /user/:id/unblock' do
+    let(:blocked_user)  { create(:user, state: 'blocked') }
     before { admin }
+
     it 'should unblock existing user' do
       put api("/users/#{user.id}/unblock", admin)
       expect(response.status).to eq(200)
@@ -805,12 +815,15 @@ describe API::API, api: true  do
     end
 
     it 'should unblock a blocked user' do
-      put api("/users/#{user.id}/block", admin)
-      expect(response.status).to eq(200)
-      expect(user.reload.state).to eq('blocked')
-      put api("/users/#{user.id}/unblock", admin)
+      put api("/users/#{blocked_user.id}/unblock", admin)
       expect(response.status).to eq(200)
-      expect(user.reload.state).to eq('active')
+      expect(blocked_user.reload.state).to eq('active')
+    end
+
+    it 'should not unblock ldap blocked users' do
+      put api("/users/#{ldap_blocked_user.id}/unblock", admin)
+      expect(response.status).to eq(403)
+      expect(ldap_blocked_user.reload.state).to eq('ldap_blocked')
     end
 
     it 'should not be available for non admin users' do
-- 
cgit v1.2.1


From d6dc088affeee4568e771e1d7894e0bcdb955af8 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
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')

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 ec67e9be1d7486199b47e19c766202a8bfdefe93 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Wed, 6 Jan 2016 05:38:52 -0200
Subject: Repair ldap_blocked state when no ldap identity exist anymore

---
 .../admin/identities_controller_spec.rb            | 26 +++++++++++++++
 spec/models/identity_spec.rb                       | 38 ++++++++++++++++++++++
 .../repair_ldap_blocked_user_service_spec.rb       | 23 +++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100644 spec/controllers/admin/identities_controller_spec.rb
 create mode 100644 spec/models/identity_spec.rb
 create mode 100644 spec/services/repair_ldap_blocked_user_service_spec.rb

(limited to 'spec')

diff --git a/spec/controllers/admin/identities_controller_spec.rb b/spec/controllers/admin/identities_controller_spec.rb
new file mode 100644
index 00000000000..c131d22a30a
--- /dev/null
+++ b/spec/controllers/admin/identities_controller_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+
+describe Admin::IdentitiesController do
+  let(:admin) { create(:admin) }
+  before { sign_in(admin) }
+
+  describe 'UPDATE identity' do
+    let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') }
+
+    it 'repairs ldap blocks' do
+      expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute)
+
+      put :update, user_id: user.username, id: user.ldap_identity.id, identity: { provider: 'twitter' }
+    end
+  end
+
+  describe 'DELETE identity' do
+    let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') }
+
+    it 'repairs ldap blocks' do
+      expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute)
+
+      delete :destroy, user_id: user.username, id: user.ldap_identity.id
+    end
+  end
+end
diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
new file mode 100644
index 00000000000..107bfc17782
--- /dev/null
+++ b/spec/models/identity_spec.rb
@@ -0,0 +1,38 @@
+# == Schema Information
+#
+# Table name: identities
+#
+#  id         :integer          not null, primary key
+#  extern_uid :string(255)
+#  provider   :string(255)
+#  user_id    :integer
+#  created_at :datetime
+#  updated_at :datetime
+#
+
+require 'spec_helper'
+
+RSpec.describe Identity, models: true do
+
+  describe 'relations' do
+    it { is_expected.to belong_to(:user) }
+  end
+
+  describe 'fields' do
+    it { is_expected.to respond_to(:provider) }
+    it { is_expected.to respond_to(:extern_uid) }
+  end
+
+  describe '#is_ldap?' do
+    let(:ldap_identity) { create(:identity, provider: 'ldapmain') }
+    let(:other_identity) { create(:identity, provider: 'twitter') }
+
+    it 'returns true if it is a ldap identity' do
+      expect(ldap_identity.is_ldap?).to be_truthy
+    end
+
+    it 'returns false if it is not a ldap identity' do
+      expect(other_identity.is_ldap?).to be_falsey
+    end
+  end
+end
diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb
new file mode 100644
index 00000000000..2a2114d038c
--- /dev/null
+++ b/spec/services/repair_ldap_blocked_user_service_spec.rb
@@ -0,0 +1,23 @@
+require 'spec_helper'
+
+describe RepairLdapBlockedUserService, services: true do
+  let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
+  let(:identity) { user.ldap_identity }
+  subject(:service) { RepairLdapBlockedUserService.new(user, identity) }
+
+  describe '#execute' do
+    it 'change to normal block after destroying last ldap identity' do
+      identity.destroy
+      service.execute
+
+      expect(user.reload).not_to be_ldap_blocked
+    end
+
+    it 'change to normal block after changing last ldap identity to another provider' do
+      identity.update_attribute(:provider, 'twitter')
+      service.execute
+
+      expect(user.reload).not_to be_ldap_blocked
+    end
+  end
+end
-- 
cgit v1.2.1


From 47e4613f4adc2d6ef4b066a87ec772ef8044bdd5 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Thu, 7 Jan 2016 14:01:01 -0200
Subject: Code style fixes and some code simplified

---
 spec/services/repair_ldap_blocked_user_service_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'spec')

diff --git a/spec/services/repair_ldap_blocked_user_service_spec.rb b/spec/services/repair_ldap_blocked_user_service_spec.rb
index 2a2114d038c..ce7d1455975 100644
--- a/spec/services/repair_ldap_blocked_user_service_spec.rb
+++ b/spec/services/repair_ldap_blocked_user_service_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
 describe RepairLdapBlockedUserService, services: true do
   let(:user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') }
   let(:identity) { user.ldap_identity }
-  subject(:service) { RepairLdapBlockedUserService.new(user, identity) }
+  subject(:service) { RepairLdapBlockedUserService.new(user) }
 
   describe '#execute' do
     it 'change to normal block after destroying last ldap identity' do
-- 
cgit v1.2.1


From ac6a10f3e88c5d2081b8638df63016089517a844 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
Date: Tue, 12 Jan 2016 12:29:10 -0200
Subject: Codestyle changes

---
 spec/models/identity_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'spec')

diff --git a/spec/models/identity_spec.rb b/spec/models/identity_spec.rb
index 107bfc17782..5afe042e154 100644
--- a/spec/models/identity_spec.rb
+++ b/spec/models/identity_spec.rb
@@ -28,11 +28,11 @@ RSpec.describe Identity, models: true do
     let(:other_identity) { create(:identity, provider: 'twitter') }
 
     it 'returns true if it is a ldap identity' do
-      expect(ldap_identity.is_ldap?).to be_truthy
+      expect(ldap_identity.ldap?).to be_truthy
     end
 
     it 'returns false if it is not a ldap identity' do
-      expect(other_identity.is_ldap?).to be_falsey
+      expect(other_identity.ldap?).to be_falsey
     end
   end
 end
-- 
cgit v1.2.1


From 3b7f34281e4d1c4ca626578ddc9a1b9eda7e7538 Mon Sep 17 00:00:00 2001
From: Tomasz Maczukin <tomasz@maczukin.pl>
Date: Wed, 13 Jan 2016 19:57:23 +0100
Subject: Modify :ci_variable factory

---
 spec/factories/ci/variables.rb      | 7 ++-----
 spec/requests/api/variables_spec.rb | 4 ++--
 2 files changed, 4 insertions(+), 7 deletions(-)

(limited to 'spec')

diff --git a/spec/factories/ci/variables.rb b/spec/factories/ci/variables.rb
index a19b9fc72f2..8f62d64411b 100644
--- a/spec/factories/ci/variables.rb
+++ b/spec/factories/ci/variables.rb
@@ -16,10 +16,7 @@
 
 FactoryGirl.define do
   factory :ci_variable, class: Ci::Variable do
-    id 10
-    key 'TEST_VARIABLE_1'
-    value 'VALUE_1'
-
-    project factory: :empty_project
+    sequence(:key) { |n| "VARIABLE_#{n}" }
+    value 'VARIABLE_VALUE'
   end
 end
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index 214d7d5a0cc..9744729ba0c 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -46,7 +46,7 @@ describe API::API, api: true do
         expect(json_response['value']).to eq(variable.value)
       end
 
-      it 'should responde with 404 Not Found if requesting non-existing variable' do
+      it 'should respond with 404 Not Found if requesting non-existing variable' do
         get api("/projects/#{project.id}/variables/non_existing_variable", user)
 
         expect(response.status).to eq(404)
@@ -84,7 +84,7 @@ describe API::API, api: true do
 
       it 'should not allow to duplicate variable key' do
         expect do
-          post api("/projects/#{project.id}/variables", user), key: 'TEST_VARIABLE_1', value: 'VALUE_2'
+          post api("/projects/#{project.id}/variables", user), key: variable.key, value: 'VALUE_2'
         end.to change{project.variables.count}.by(0)
 
         expect(response.status).to eq(400)
-- 
cgit v1.2.1


From dd6fc01ff8a073880b67a323a547edeb5d63f167 Mon Sep 17 00:00:00 2001
From: Gabriel Mazetto <gabriel@gitlab.com>
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')

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 2c6b11198aa636b7e54a55bf63d0daabcfb716d8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 17 Dec 2015 09:22:02 +0100
Subject: Add CI build artifacts tarball as a spec fixture

---
 spec/fixtures/ci_build_artifacts.tar.gz | Bin 0 -> 34085 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 spec/fixtures/ci_build_artifacts.tar.gz

(limited to 'spec')

diff --git a/spec/fixtures/ci_build_artifacts.tar.gz b/spec/fixtures/ci_build_artifacts.tar.gz
new file mode 100644
index 00000000000..a60de1bf95e
Binary files /dev/null and b/spec/fixtures/ci_build_artifacts.tar.gz differ
-- 
cgit v1.2.1


From f5d530865875440d69217cf249715bffaa3d11b8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 87df1df3cd1a38bbb523d365b229e5e03f890788 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 28 Dec 2015 10:54:22 +0100
Subject: Seed db with CI build artifacts using a zip archive

---
 spec/fixtures/ci_build_artifacts.tar.gz | Bin 34085 -> 0 bytes
 spec/fixtures/ci_build_artifacts.zip    | Bin 0 -> 105373 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 spec/fixtures/ci_build_artifacts.tar.gz
 create mode 100644 spec/fixtures/ci_build_artifacts.zip

(limited to 'spec')

diff --git a/spec/fixtures/ci_build_artifacts.tar.gz b/spec/fixtures/ci_build_artifacts.tar.gz
deleted file mode 100644
index a60de1bf95e..00000000000
Binary files a/spec/fixtures/ci_build_artifacts.tar.gz and /dev/null differ
diff --git a/spec/fixtures/ci_build_artifacts.zip b/spec/fixtures/ci_build_artifacts.zip
new file mode 100644
index 00000000000..ec47005ce85
Binary files /dev/null and b/spec/fixtures/ci_build_artifacts.zip differ
-- 
cgit v1.2.1


From 9e0e9342a47022a9caaa4a5596ec3ddb91fddc58 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 28 Dec 2015 11:21:01 +0100
Subject: Rename method that returns url to CI build artifacts download

---
 spec/models/build_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'spec')

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 1c22e3cb7c4..85cba9c3e57 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -368,8 +368,8 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :download_url do
-    subject { build.download_url }
+  describe :artifacts_download_url do
+    subject { build.artifacts_download_url }
 
     it "should be nil if artifact doesn't exist" do
       build.update_attributes(artifacts_file: nil)
-- 
cgit v1.2.1


From 8eeed761a9c25ea8ccfc347fbd3f5894b5957d9e Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 28 Dec 2015 11:43:15 +0100
Subject: Update specs for CI Build, add `artifacts?` method

`artifacts?` method checks if artifacts archive is available.
---
 spec/models/build_spec.rb | 60 +++++++++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 26 deletions(-)

(limited to 'spec')

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 85cba9c3e57..33e0eb7d5d7 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -1,28 +1,3 @@
-# == Schema Information
-#
-# Table name: builds
-#
-#  id                 :integer          not null, primary key
-#  project_id         :integer
-#  status             :string(255)
-#  finished_at        :datetime
-#  trace              :text
-#  created_at         :datetime
-#  updated_at         :datetime
-#  started_at         :datetime
-#  runner_id          :integer
-#  commit_id          :integer
-#  coverage           :float
-#  commands           :text
-#  job_id             :integer
-#  name               :string(255)
-#  deploy             :boolean          default(FALSE)
-#  options            :text
-#  allow_failure      :boolean          default(FALSE), not null
-#  stage              :string(255)
-#  trigger_request_id :integer
-#
-
 require 'spec_helper'
 
 describe Ci::Build, models: true do
@@ -376,13 +351,46 @@ describe Ci::Build, models: true do
       is_expected.to be_nil
     end
 
-    it 'should be nil if artifact exist' do
+    it 'should not be nil if artifact exist' do
       gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
       build.update_attributes(artifacts_file: gif)
       is_expected.to_not be_nil
     end
   end
 
+  describe :artifacts_browse_url do
+    subject { build.artifacts_browse_url }
+
+    it "should be nil if artifact doesn't exist" do
+      build.update_attributes(artifacts_file: nil)
+      is_expected.to be_nil
+    end
+
+    it 'should not be nil if artifact exist' do
+      gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
+      build.update_attributes(artifacts_file: gif)
+      is_expected.to_not be_nil
+    end
+  end
+
+  describe :artifacts? do
+    subject { build.artifacts? }
+
+    context 'artifacts archive does not exist' do
+      before { build.update_attributes(artifacts_file: nil) }
+      it { is_expected.to be_falsy }
+    end
+
+    context 'artifacts archive exists' do
+      before do
+        gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
+        build.update_attributes(artifacts_file: gif)
+      end
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
   describe :repo_url do
     let(:build) { FactoryGirl.create :ci_build }
     let(:project) { build.project }
-- 
cgit v1.2.1


From 5ff7ec42dc8759717c485478261128d61ea70b9a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 28 Dec 2015 12:06:27 +0100
Subject: Add method that checks if artifacts browser is supported

This is needed because of backward compatibility. Previously artifacts
archive had `.tar.gz` format, but artifacts browser requires ZIP format
now.
---
 spec/models/build_spec.rb | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

(limited to 'spec')

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 33e0eb7d5d7..108d7d5ff01 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -391,6 +391,29 @@ describe Ci::Build, models: true do
     end
   end
 
+
+  describe :artifacts_browser_supported? do
+    subject { build.artifacts_browser_supported? }
+    before do
+      file = fixture_file_upload(archive_file, archive_type)
+      build.update_attributes(artifacts_file: file)
+    end
+
+    context 'artifacts archive is not a zip file' do
+      let(:archive_file) { Rails.root + 'spec/fixtures/banana_sample.gif' }
+      let(:archive_type) { 'image/gif' }
+
+      it { is_expected.to be_falsy }
+    end
+
+    context 'artifacts archive is a zip file' do
+      let(:archive_file) { Rails.root + 'spec/fixtures/ci_build_artifacts.zip' }
+      let(:archive_type) { 'application/zip' }
+
+      it { is_expected.to be_truthy }
+    end
+  end
+
   describe :repo_url do
     let(:build) { FactoryGirl.create :ci_build }
     let(:project) { build.project }
-- 
cgit v1.2.1


From ece114e630f11a7d244ab55a0ffd49a1d2cbcb94 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 28 Dec 2015 12:34:47 +0100
Subject: Update artifacts download specs

---
 spec/features/builds_spec.rb | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

(limited to 'spec')

diff --git a/spec/features/builds_spec.rb b/spec/features/builds_spec.rb
index 240e56839df..d37bd103714 100644
--- a/spec/features/builds_spec.rb
+++ b/spec/features/builds_spec.rb
@@ -80,7 +80,11 @@ describe "Builds" do
         visit namespace_project_build_path(@project.namespace, @project, @build)
       end
 
-      it { expect(page).to have_content 'Download artifacts' }
+      it 'has button to download artifacts' do
+        page.within('.artifacts') do
+          expect(page).to have_content 'Download'
+        end
+      end
     end
   end
 
@@ -111,7 +115,7 @@ describe "Builds" do
     before do
       @build.update_attributes(artifacts_file: artifacts_file)
       visit namespace_project_build_path(@project.namespace, @project, @build)
-      click_link 'Download artifacts'
+      page.within('.artifacts') { click_link 'Download' }
     end
 
     it { expect(page.response_headers['Content-Type']).to eq(artifacts_file.content_type) }
-- 
cgit v1.2.1


From 662f4b9e1dec8e461c4ea8da3ccc46a259d9d205 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 30 Dec 2015 14:41:44 +0100
Subject: Add artifacts metadata uploader filed

Artifacts metadata field will be used to store a filename of gzipped
file containing metadata definition for given artifacts archive.
---
 spec/fixtures/ci_build_artifacts_metadata.gz | Bin 0 -> 279 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 spec/fixtures/ci_build_artifacts_metadata.gz

(limited to 'spec')

diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz
new file mode 100644
index 00000000000..bd0c8ada20a
Binary files /dev/null and b/spec/fixtures/ci_build_artifacts_metadata.gz differ
-- 
cgit v1.2.1


From e3ef0ac8f44118465cf5831982d2051d0986cda8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 31 Dec 2015 09:24:59 +0100
Subject: Update artifacts metadata fixture

---
 spec/fixtures/ci_build_artifacts_metadata.gz | Bin 279 -> 242 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)

(limited to 'spec')

diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz
index bd0c8ada20a..82d6a79c72f 100644
Binary files a/spec/fixtures/ci_build_artifacts_metadata.gz and b/spec/fixtures/ci_build_artifacts_metadata.gz differ
-- 
cgit v1.2.1


From 3de8a4620a70c886c815576dc0a30a745cbb8ccb Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 cfffc9eff2c394259000090fb8c4c116863c9199 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Jan 2016 13:05:44 +0000
Subject: Update build specs for artifacts browser support

---
 spec/models/build_spec.rb | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

(limited to 'spec')

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 108d7d5ff01..ca96e827e04 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -394,21 +394,20 @@ describe Ci::Build, models: true do
 
   describe :artifacts_browser_supported? do
     subject { build.artifacts_browser_supported? }
-    before do
-      file = fixture_file_upload(archive_file, archive_type)
-      build.update_attributes(artifacts_file: file)
-    end
-
-    context 'artifacts archive is not a zip file' do
-      let(:archive_file) { Rails.root + 'spec/fixtures/banana_sample.gif' }
-      let(:archive_type) { 'image/gif' }
-
+    context 'artifacts metadata does not exist' do
       it { is_expected.to be_falsy }
     end
 
-    context 'artifacts archive is a zip file' do
-      let(:archive_file) { Rails.root + 'spec/fixtures/ci_build_artifacts.zip' }
-      let(:archive_type) { 'application/zip' }
+    context 'artifacts archive is a zip file and metadata exists' do
+      before do
+        fixture_dir = Rails.root + 'spec/fixtures/'
+        archive = fixture_file_upload(fixture_dir + 'ci_build_artifacts.zip',
+                                      'application/zip')
+        metadata = fixture_file_upload(fixture_dir + 'ci_build_artifacts_metadata.gz',
+                                       'application/x-gzip')
+        build.update_attributes(artifacts_file: archive)
+        build.update_attributes(artifacts_metadata: metadata)
+      end
 
       it { is_expected.to be_truthy }
     end
-- 
cgit v1.2.1


From 387b27813d1d496c015f4f174812b4761c32648d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
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/fixtures/ci_build_artifacts_metadata.gz        | Bin 242 -> 309 bytes
 spec/lib/gitlab/ci/build/artifacts/metadata_spec.rb |  15 +++++++++++++++
 2 files changed, 15 insertions(+)

(limited to 'spec')

diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz
index 82d6a79c72f..c394f83bf87 100644
Binary files a/spec/fixtures/ci_build_artifacts_metadata.gz and b/spec/fixtures/ci_build_artifacts_metadata.gz differ
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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 f80d7a868e83f7cbba2d0c42ed9464552d9c7a0b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 11 Jan 2016 11:18:21 +0100
Subject: Update build model specs

---
 spec/models/build_spec.rb | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

(limited to 'spec')

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index ca96e827e04..0e13456723d 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -361,14 +361,13 @@ describe Ci::Build, models: true do
   describe :artifacts_browse_url do
     subject { build.artifacts_browse_url }
 
-    it "should be nil if artifact doesn't exist" do
-      build.update_attributes(artifacts_file: nil)
+    it "should be nil if artifacts browser is unsupported" do
+      allow(build).to receive(:artifacts_browser_supported?).and_return(false)
       is_expected.to be_nil
     end
 
-    it 'should not be nil if artifact exist' do
-      gif = fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif')
-      build.update_attributes(artifacts_file: gif)
+    it 'should not be nil if artifacts browser is supported' do
+      allow(build).to receive(:artifacts_browser_supported?).and_return(true)
       is_expected.to_not be_nil
     end
   end
-- 
cgit v1.2.1


From 2be76355caa579d444c8e3c0d25563eb9778bfb2 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 12 Jan 2016 10:04:26 +0100
Subject: Support only valid UTF-8 paths in build artifacts browser

---
 spec/fixtures/ci_build_artifacts.zip         | Bin 105373 -> 106365 bytes
 spec/fixtures/ci_build_artifacts_metadata.gz | Bin 309 -> 415 bytes
 2 files changed, 0 insertions(+), 0 deletions(-)

(limited to 'spec')

diff --git a/spec/fixtures/ci_build_artifacts.zip b/spec/fixtures/ci_build_artifacts.zip
index ec47005ce85..dae976d918e 100644
Binary files a/spec/fixtures/ci_build_artifacts.zip and b/spec/fixtures/ci_build_artifacts.zip differ
diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz
index c394f83bf87..fe9d4c8c661 100644
Binary files a/spec/fixtures/ci_build_artifacts_metadata.gz and b/spec/fixtures/ci_build_artifacts_metadata.gz differ
-- 
cgit v1.2.1


From cf00a808cc9896093be209dc5d6bfbea93b6226b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
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')

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 e8995f9fd5c12882eafcf3766627f64a3d6f623d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 12 Jan 2016 14:39:15 +0100
Subject: Modify artifacts upload API endpoint, add artifacts metadata

---
 spec/requests/ci/api/builds_spec.rb | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

(limited to 'spec')

diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index c27e87c4acc..4eb5f2e6828 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -210,6 +210,31 @@ describe Ci::API::API do
             end
           end
 
+          context "should post artifacts metadata" do
+            let!(:artifacts) { file_upload }
+            let!(:metadata) { file_upload2 }
+
+            before do
+              build.run!
+
+              post_data = {
+                'file.path' => artifacts.path,
+                'file.name' => artifacts.original_filename,
+                'metadata.path' => metadata.path,
+                'metadata.name' => metadata.original_filename
+              }
+
+              post post_url, post_data, headers_with_token
+            end
+
+            it 'stores artifacts and artifacts metadata' do
+              expect(response.status).to eq(201)
+              expect(json_response['artifacts_file']['filename']).to eq(artifacts.original_filename)
+              expect(json_response['artifacts_metadata']['filename']).to eq(metadata.original_filename)
+            end
+          end
+
+
           context "should fail to post too large artifact" do
             before do
               build.run!
-- 
cgit v1.2.1


From 0b946029a1fb429db39fbec0cddccf40f7e2aa08 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 13 Jan 2016 09:56:05 +0100
Subject: Update build artifacts API

We do not want to allow runners to upload a metadata file. This needs to
be generated by Workhorse only.
---
 spec/requests/ci/api/builds_spec.rb | 48 +++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 13 deletions(-)

(limited to 'spec')

diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index 4eb5f2e6828..f47ffb00e33 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -210,27 +210,49 @@ describe Ci::API::API do
             end
           end
 
-          context "should post artifacts metadata" do
+          context 'should post artifacts file and metadata file' do
             let!(:artifacts) { file_upload }
             let!(:metadata) { file_upload2 }
 
+            let(:stored_artifacts_file) { build.reload.artifacts_file.file }
+            let(:stored_metadata_file) { build.reload.artifacts_metadata.file }
+
             before do
               build.run!
+              post(post_url, post_data, headers_with_token)
+            end
 
-              post_data = {
-                'file.path' => artifacts.path,
-                'file.name' => artifacts.original_filename,
-                'metadata.path' => metadata.path,
-                'metadata.name' => metadata.original_filename
-              }
-
-              post post_url, post_data, headers_with_token
+            context 'post data accelerated by workhorse is correct' do
+              let(:post_data) do
+                { 'file.path' => artifacts.path,
+                  'file.name' => artifacts.original_filename,
+                  'metadata.path' => metadata.path,
+                  'metadata.name' => metadata.original_filename }
+              end
+
+              it 'responds with valid status' do
+                expect(response.status).to eq(201)
+              end
+
+              it 'stores artifacts and artifacts metadata' do
+                expect(stored_artifacts_file.original_filename).to eq(artifacts.original_filename)
+                expect(stored_metadata_file.original_filename).to eq(metadata.original_filename)
+              end
             end
 
-            it 'stores artifacts and artifacts metadata' do
-              expect(response.status).to eq(201)
-              expect(json_response['artifacts_file']['filename']).to eq(artifacts.original_filename)
-              expect(json_response['artifacts_metadata']['filename']).to eq(metadata.original_filename)
+            context 'runner sends metadata file' do
+              let(:post_data) do
+                { 'file' => artifacts, 'metadata' => metadata }
+              end
+
+              it 'is expected to respond with forbbiden' do
+                expect(response.status).to eq(403)
+              end
+
+              it 'does not store artifacts or metadata' do
+                expect(stored_artifacts_file).to be_nil
+                expect(stored_metadata_file).to be_nil
+              end
             end
           end
 
-- 
cgit v1.2.1


From 154b8ceba4ac2d92a2387ad50d7f2b4ed5b2dd8a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 13 Jan 2016 14:02:36 +0100
Subject: Refactor build artifacts upload API endpoint

---
 spec/requests/ci/api/builds_spec.rb | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

(limited to 'spec')

diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index f47ffb00e33..648ea0d5f50 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -240,17 +240,16 @@ describe Ci::API::API do
               end
             end
 
-            context 'runner sends metadata file' do
+            context 'no artifacts file in post data' do
               let(:post_data) do
-                { 'file' => artifacts, 'metadata' => metadata }
+                { 'metadata' => metadata }
               end
 
-              it 'is expected to respond with forbbiden' do
-                expect(response.status).to eq(403)
+              it 'is expected to respond with bad request' do
+                expect(response.status).to eq(400)
               end
 
-              it 'does not store artifacts or metadata' do
-                expect(stored_artifacts_file).to be_nil
+              it 'does not store metadata' do
                 expect(stored_metadata_file).to be_nil
               end
             end
-- 
cgit v1.2.1


From 6b0a43aff36f0bbb9050b3c04155a3ccd9c1a75b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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 <grzesiek.bizon@gmail.com>
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')

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