From 589b2db06ca2ca2bc3e5d9e56968e3609f9e4626 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Tue, 23 Apr 2019 16:27:01 +0200
Subject: Setup Phabricator import

This sets up all the basics for importing Phabricator tasks into
GitLab issues.

To import all tasks from a Phabricator instance into GitLab, we'll
import all of them into a new project that will have its repository
disabled.

The import is hooked into a regular ProjectImport setup, but similar
to the GitHub parallel importer takes care of all the imports itself.

In this iteration, we're importing each page of tasks in a separate
sidekiq job.

The first thing we do when requesting a new page of tasks is schedule
the next page to be imported. But to avoid deadlocks, we only allow a
single job per worker type to run at the same time.

For now we're only importing basic Issue information, this should be
extended to richer information.
---
 .../gitlab/phabricator_import/base_worker_spec.rb  | 74 ++++++++++++++++++++
 .../gitlab/phabricator_import/cache/map_spec.rb    | 66 ++++++++++++++++++
 .../phabricator_import/conduit/client_spec.rb      | 59 ++++++++++++++++
 .../phabricator_import/conduit/maniphest_spec.rb   | 39 +++++++++++
 .../phabricator_import/conduit/response_spec.rb    | 79 ++++++++++++++++++++++
 .../conduit/tasks_response_spec.rb                 | 27 ++++++++
 .../phabricator_import/import_tasks_worker_spec.rb | 16 +++++
 .../lib/gitlab/phabricator_import/importer_spec.rb | 32 +++++++++
 .../phabricator_import/issues/importer_spec.rb     | 53 +++++++++++++++
 .../issues/task_importer_spec.rb                   | 54 +++++++++++++++
 .../phabricator_import/project_creator_spec.rb     | 58 ++++++++++++++++
 .../phabricator_import/representation/task_spec.rb | 33 +++++++++
 .../gitlab/phabricator_import/worker_state_spec.rb | 46 +++++++++++++
 13 files changed, 636 insertions(+)
 create mode 100644 spec/lib/gitlab/phabricator_import/base_worker_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/cache/map_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/conduit/client_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/conduit/response_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/importer_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/issues/importer_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/project_creator_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/representation/task_spec.rb
 create mode 100644 spec/lib/gitlab/phabricator_import/worker_state_spec.rb

(limited to 'spec/lib/gitlab/phabricator_import')

diff --git a/spec/lib/gitlab/phabricator_import/base_worker_spec.rb b/spec/lib/gitlab/phabricator_import/base_worker_spec.rb
new file mode 100644
index 00000000000..d46d908a3e3
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/base_worker_spec.rb
@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::BaseWorker do
+  let(:subclass) do
+    # Creating an anonymous class for a worker is complicated, as we generate the
+    # queue name from the class name.
+    Gitlab::PhabricatorImport::ImportTasksWorker
+  end
+
+  describe '.schedule' do
+    let(:arguments) { %w[project_id the_next_page] }
+
+    it 'schedules the job' do
+      expect(subclass).to receive(:perform_async).with(*arguments)
+
+      subclass.schedule(*arguments)
+    end
+
+    it 'counts the scheduled job', :clean_gitlab_redis_shared_state do
+      state = Gitlab::PhabricatorImport::WorkerState.new('project_id')
+
+      allow(subclass).to receive(:remove_job) # otherwise the job is removed before we saw it
+
+      expect { subclass.schedule(*arguments) }.to change { state.running_count }.by(1)
+    end
+  end
+
+  describe '#perform' do
+    let(:project) { create(:project, :import_started, import_url: "https://a.phab.instance") }
+    let(:worker) { subclass.new }
+    let(:state) { Gitlab::PhabricatorImport::WorkerState.new(project.id) }
+
+    before do
+      allow(worker).to receive(:import)
+    end
+
+    it 'does not break for a non-existing project' do
+      expect { worker.perform('not a thing') }.not_to raise_error
+    end
+
+    it 'does not do anything when the import is not in progress' do
+      project = create(:project, :import_failed)
+
+      expect(worker).not_to receive(:import)
+
+      worker.perform(project.id)
+    end
+
+    it 'calls import for the project' do
+      expect(worker).to receive(:import).with(project, 'other_arg')
+
+      worker.perform(project.id, 'other_arg')
+    end
+
+    it 'marks the project as imported if there was only one job running' do
+      worker.perform(project.id)
+
+      expect(project.import_state.reload).to be_finished
+    end
+
+    it 'does not mark the job as finished when there are more scheduled jobs' do
+      2.times { state.add_job }
+
+      worker.perform(project.id)
+
+      expect(project.import_state.reload).to be_in_progress
+    end
+
+    it 'decrements the job counter' do
+      expect { worker.perform(project.id) }.to change { state.running_count }.by(-1)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/cache/map_spec.rb b/spec/lib/gitlab/phabricator_import/cache/map_spec.rb
new file mode 100644
index 00000000000..52c7a02219f
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/cache/map_spec.rb
@@ -0,0 +1,66 @@
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Cache::Map, :clean_gitlab_redis_cache do
+  set(:project) { create(:project) }
+  let(:redis) { Gitlab::Redis::Cache }
+  subject(:map) { described_class.new(project) }
+
+  describe '#get_gitlab_model' do
+    it 'returns nil if there was nothing cached for the phabricator id' do
+      expect(map.get_gitlab_model('does not exist')).to be_nil
+    end
+
+    it 'returns the object if it was set in redis' do
+      issue = create(:issue, project: project)
+      set_in_redis('exists', issue)
+
+      expect(map.get_gitlab_model('exists')).to eq(issue)
+    end
+
+    it 'extends the TTL for the cache key' do
+      set_in_redis('extend', create(:issue, project: project)) do |redis|
+        redis.expire(cache_key('extend'), 10.seconds.to_i)
+      end
+
+      map.get_gitlab_model('extend')
+
+      ttl = redis.with { |redis| redis.ttl(cache_key('extend')) }
+
+      expect(ttl).to be > 10.seconds
+    end
+  end
+
+  describe '#set_gitlab_model' do
+    around do |example|
+      Timecop.freeze { example.run }
+    end
+
+    it 'sets the class and id in redis with a ttl' do
+      issue = create(:issue, project: project)
+
+      map.set_gitlab_model(issue, 'it is set')
+
+      set_data, ttl = redis.with do |redis|
+        redis.pipelined do |p|
+          p.mapped_hmget(cache_key('it is set'), :classname, :database_id)
+          p.ttl(cache_key('it is set'))
+        end
+      end
+
+      expect(set_data).to eq({ classname: 'Issue', database_id: issue.id.to_s })
+      expect(ttl).to be_within(1.second).of(StuckImportJobsWorker::IMPORT_JOBS_EXPIRATION)
+    end
+  end
+
+  def set_in_redis(key, object)
+    redis.with do |redis|
+      redis.mapped_hmset(cache_key(key),
+                         { classname: object.class, database_id: object.id })
+      yield(redis) if block_given?
+    end
+  end
+
+  def cache_key(phabricator_id)
+    subject.__send__(:cache_key_for_phabricator_id, phabricator_id)
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb
new file mode 100644
index 00000000000..542b3cd060f
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/conduit/client_spec.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Conduit::Client do
+  let(:client) do
+    described_class.new('https://see-ya-later.phabricator', 'api-token')
+  end
+
+  describe '#get' do
+    it 'performs and parses a request' do
+      params = { some: 'extra', values: %w[are passed] }
+      stub_valid_request(params)
+
+      response = client.get('test', params: params)
+
+      expect(response).to be_a(Gitlab::PhabricatorImport::Conduit::Response)
+      expect(response).to be_success
+    end
+
+    it 'wraps request errors in an `ApiError`' do
+      stub_timeout
+
+      expect { client.get('test') }.to raise_error(Gitlab::PhabricatorImport::Conduit::ApiError)
+    end
+
+    it 'raises response error' do
+      stub_error_response
+
+      expect { client.get('test') }
+        .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /has the wrong length/)
+    end
+  end
+
+  def stub_valid_request(params = {})
+    WebMock.stub_request(
+      :get, 'https://see-ya-later.phabricator/api/test'
+    ).with(
+      body: CGI.unescape(params.reverse_merge('api.token' => 'api-token').to_query)
+    ).and_return(
+      status: 200,
+      body: fixture_file('phabricator_responses/maniphest.search.json')
+    )
+  end
+
+  def stub_timeout
+    WebMock.stub_request(
+      :get, 'https://see-ya-later.phabricator/api/test'
+    ).to_timeout
+  end
+
+  def stub_error_response
+    WebMock.stub_request(
+      :get, 'https://see-ya-later.phabricator/api/test'
+    ).and_return(
+      status: 200,
+      body: fixture_file('phabricator_responses/auth_failed.json')
+    )
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb
new file mode 100644
index 00000000000..0d7714649b9
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/conduit/maniphest_spec.rb
@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Conduit::Maniphest do
+  let(:maniphest) do
+    described_class.new(phabricator_url: 'https://see-ya-later.phabricator', api_token: 'api-token')
+  end
+
+  describe '#tasks' do
+    let(:fake_client) { double('Phabricator client') }
+
+    before do
+      allow(maniphest).to receive(:client).and_return(fake_client)
+    end
+
+    it 'calls the api with the correct params' do
+      expected_params = {
+        after: '123',
+        attachments: {
+          projects: 1, subscribers: 1, columns: 1
+        }
+      }
+
+      expect(fake_client).to receive(:get).with('maniphest.search',
+                                                params: expected_params)
+
+      maniphest.tasks(after: '123')
+    end
+
+    it 'returns a parsed response' do
+      response = Gitlab::PhabricatorImport::Conduit::Response
+                   .new(fixture_file('phabricator_responses/maniphest.search.json'))
+
+      allow(fake_client).to receive(:get).and_return(response)
+
+      expect(maniphest.tasks).to be_a(Gitlab::PhabricatorImport::Conduit::TasksResponse)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb
new file mode 100644
index 00000000000..a8596968f14
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/conduit/response_spec.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Conduit::Response do
+  let(:response) { described_class.new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json')))}
+  let(:error_response) { described_class.new(JSON.parse(fixture_file('phabricator_responses/auth_failed.json'))) }
+
+  describe '.parse!' do
+    it 'raises a ResponseError if the http response was not successfull' do
+      fake_response = double(:http_response, success?: false, status: 401)
+
+      expect { described_class.parse!(fake_response) }
+        .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /responded with 401/)
+    end
+
+    it 'raises a ResponseError if the response contained a Phabricator error' do
+      fake_response = double(:http_response,
+                             success?: true,
+                             status: 200,
+                             body: fixture_file('phabricator_responses/auth_failed.json'))
+
+      expect { described_class.parse!(fake_response) }
+        .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /ERR-INVALID-AUTH: API token/)
+    end
+
+    it 'raises a ResponseError if JSON parsing failed' do
+      fake_response = double(:http_response,
+                       success?: true,
+                       status: 200,
+                       body: 'This is no JSON')
+
+      expect { described_class.parse!(fake_response) }
+        .to raise_error(Gitlab::PhabricatorImport::Conduit::ResponseError, /unexpected token at/)
+    end
+
+    it 'returns a parsed response for valid input' do
+      fake_response = double(:http_response,
+                       success?: true,
+                       status: 200,
+                       body: fixture_file('phabricator_responses/maniphest.search.json'))
+
+      expect(described_class.parse!(fake_response)).to be_a(described_class)
+    end
+  end
+
+  describe '#success?' do
+    it { expect(response).to be_success }
+    it { expect(error_response).not_to be_success }
+  end
+
+  describe '#error_code' do
+    it { expect(error_response.error_code).to eq('ERR-INVALID-AUTH') }
+    it { expect(response.error_code).to be_nil }
+  end
+
+  describe '#error_info' do
+    it 'returns the correct error info' do
+      expected_message = 'API token "api-token" has the wrong length. API tokens should be 32 characters long.'
+
+      expect(error_response.error_info).to eq(expected_message)
+    end
+
+    it { expect(response.error_info).to be_nil }
+  end
+
+  describe '#data' do
+    it { expect(error_response.data).to be_nil }
+    it { expect(response.data).to be_an(Array) }
+  end
+
+  describe '#pagination' do
+    it { expect(error_response.pagination).to be_nil }
+
+    it 'builds the pagination correctly' do
+      expect(response.pagination).to be_a(Gitlab::PhabricatorImport::Conduit::Pagination)
+      expect(response.pagination.next_page).to eq('284')
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb b/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb
new file mode 100644
index 00000000000..4b4c2a6276e
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/conduit/tasks_response_spec.rb
@@ -0,0 +1,27 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Conduit::TasksResponse do
+  let(:conduit_response) do
+    Gitlab::PhabricatorImport::Conduit::Response
+      .new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json')))
+  end
+
+  subject(:response) { described_class.new(conduit_response) }
+
+  describe '#pagination' do
+    it 'delegates to the conduit reponse' do
+      expect(response.pagination).to eq(conduit_response.pagination)
+    end
+  end
+
+  describe '#tasks' do
+    it 'builds the correct tasks representation' do
+      tasks = response.tasks
+
+      titles = tasks.map(&:issue_attributes).map { |attrs| attrs[:title] }
+
+      expect(titles).to contain_exactly('Things are slow', 'Things are broken')
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb b/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb
new file mode 100644
index 00000000000..1e38ef8aaa5
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/import_tasks_worker_spec.rb
@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::ImportTasksWorker do
+  describe '#perform' do
+    it 'calls the correct importer' do
+      project = create(:project, :import_started, import_url: "https://the.phab.ulr")
+      fake_importer = instance_double(Gitlab::PhabricatorImport::Issues::Importer)
+
+      expect(Gitlab::PhabricatorImport::Issues::Importer).to receive(:new).with(project).and_return(fake_importer)
+      expect(fake_importer).to receive(:execute)
+
+      described_class.new.perform(project.id)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/importer_spec.rb b/spec/lib/gitlab/phabricator_import/importer_spec.rb
new file mode 100644
index 00000000000..bf14010a187
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/importer_spec.rb
@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Importer do
+  it { expect(described_class).to be_async }
+
+  it "acts like it's importing repositories" do
+    expect(described_class).to be_imports_repository
+  end
+
+  describe '#execute' do
+    let(:project) { create(:project, :import_scheduled) }
+    subject(:importer) { described_class.new(project) }
+
+    it 'sets a custom jid that will be kept up to date' do
+      expect { importer.execute }.to change { project.import_state.reload.jid }
+    end
+
+    it 'starts importing tasks' do
+      expect(Gitlab::PhabricatorImport::ImportTasksWorker).to receive(:schedule).with(project.id)
+
+      importer.execute
+    end
+
+    it 'marks the import as failed when something goes wrong' do
+      allow(importer).to receive(:schedule_first_tasks_page).and_raise('Stuff is broken')
+
+      importer.execute
+
+      expect(project.import_state).to be_failed
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb b/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb
new file mode 100644
index 00000000000..2412cf76f79
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/issues/importer_spec.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Issues::Importer do
+  set(:project) { create(:project) }
+
+  let(:response) do
+    Gitlab::PhabricatorImport::Conduit::TasksResponse.new(
+      Gitlab::PhabricatorImport::Conduit::Response
+        .new(JSON.parse(fixture_file('phabricator_responses/maniphest.search.json')))
+    )
+  end
+
+  subject(:importer) { described_class.new(project, nil) }
+
+  before do
+    client = instance_double(Gitlab::PhabricatorImport::Conduit::Maniphest)
+
+    allow(client).to receive(:tasks).and_return(response)
+    allow(importer).to receive(:client).and_return(client)
+  end
+
+  describe '#execute' do
+    it 'imports each task in the response' do
+      response.tasks.each do |task|
+        task_importer = instance_double(Gitlab::PhabricatorImport::Issues::TaskImporter)
+
+        expect(task_importer).to receive(:execute)
+        expect(Gitlab::PhabricatorImport::Issues::TaskImporter)
+          .to receive(:new).with(project, task)
+                .and_return(task_importer)
+      end
+
+      importer.execute
+    end
+
+    it 'schedules the next batch if there is one' do
+      expect(Gitlab::PhabricatorImport::ImportTasksWorker)
+        .to receive(:schedule).with(project.id, response.pagination.next_page)
+
+      importer.execute
+    end
+
+    it 'does not reschedule when there is no next page' do
+      allow(response.pagination).to receive(:has_next_page?).and_return(false)
+
+      expect(Gitlab::PhabricatorImport::ImportTasksWorker)
+        .not_to receive(:schedule)
+
+      importer.execute
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb b/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb
new file mode 100644
index 00000000000..1625604e754
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/issues/task_importer_spec.rb
@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Issues::TaskImporter do
+  set(:project) { create(:project) }
+  let(:task) do
+    Gitlab::PhabricatorImport::Representation::Task.new(
+      {
+        'phid' => 'the-phid',
+        'fields' => {
+          'name' => 'Title',
+          'description' => {
+            'raw' => '# This is markdown\n it can contain more text.'
+          },
+          'dateCreated' => '1518688921',
+          'dateClosed' => '1518789995'
+        }
+      }
+    )
+  end
+
+  describe '#execute' do
+    it 'creates the issue with the expected attributes' do
+      issue = described_class.new(project, task).execute
+
+      expect(issue.project).to eq(project)
+      expect(issue).to be_persisted
+      expect(issue.author).to eq(User.ghost)
+      expect(issue.title).to eq('Title')
+      expect(issue.description).to eq('# This is markdown\n it can contain more text.')
+      expect(issue).to be_closed
+      expect(issue.created_at).to eq(Time.at(1518688921))
+      expect(issue.closed_at).to eq(Time.at(1518789995))
+    end
+
+    it 'does not recreate the issue when called multiple times' do
+      expect { described_class.new(project, task).execute }
+        .to change { project.issues.reload.size }.from(0).to(1)
+      expect { described_class.new(project, task).execute }
+        .not_to change { project.issues.reload.size }
+    end
+
+    it 'does not trigger a save when the object did not change' do
+      existing_issue = create(:issue,
+                              task.issue_attributes.merge(author: User.ghost))
+      importer = described_class.new(project, task)
+      allow(importer).to receive(:issue).and_return(existing_issue)
+
+      expect(existing_issue).not_to receive(:save!)
+
+      importer.execute
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/project_creator_spec.rb b/spec/lib/gitlab/phabricator_import/project_creator_spec.rb
new file mode 100644
index 00000000000..e9455b866ac
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/project_creator_spec.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::ProjectCreator do
+  let(:user) { create(:user) }
+  let(:params) do
+    { path: 'new-phab-import',
+      phabricator_server_url: 'http://phab.example.com',
+      api_token: 'the-token' }
+  end
+  subject(:creator) { described_class.new(user, params) }
+
+  describe '#execute' do
+    it 'creates a project correctly and schedule an import' do
+      expect_next_instance_of(Gitlab::PhabricatorImport::Importer) do |importer|
+        expect(importer).to receive(:execute)
+      end
+
+      project = creator.execute
+
+      expect(project).to be_persisted
+      expect(project).to be_import
+      expect(project.import_type).to eq('phabricator')
+      expect(project.import_data.credentials).to match(a_hash_including(api_token: 'the-token'))
+      expect(project.import_data.data).to match(a_hash_including('phabricator_url' => 'http://phab.example.com'))
+      expect(project.import_url).to eq(Project::UNKNOWN_IMPORT_URL)
+      expect(project.namespace).to eq(user.namespace)
+    end
+
+    context 'when import params are missing' do
+      let(:params) do
+        { path: 'new-phab-import',
+          phabricator_server_url: 'http://phab.example.com',
+          api_token: '' }
+      end
+
+      it 'returns nil' do
+        expect(creator.execute).to be_nil
+      end
+    end
+
+    context 'when import params are invalid' do
+      let(:params) do
+        { path: 'new-phab-import',
+          namespace_id: '-1',
+          phabricator_server_url: 'http://phab.example.com',
+          api_token: 'the-token' }
+      end
+
+      it 'returns an unpersisted project' do
+        project = creator.execute
+
+        expect(project).not_to be_persisted
+        expect(project).not_to be_valid
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/representation/task_spec.rb b/spec/lib/gitlab/phabricator_import/representation/task_spec.rb
new file mode 100644
index 00000000000..dfbd8c546eb
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/representation/task_spec.rb
@@ -0,0 +1,33 @@
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::Representation::Task do
+  subject(:task) do
+    described_class.new(
+      {
+        'phid' => 'the-phid',
+        'fields' => {
+          'name' => 'Title'.ljust(257, '.'), # A string padded to 257 chars
+          'description' => {
+            'raw' => '# This is markdown\n it can contain more text.'
+          },
+          'dateCreated' => '1518688921',
+          'dateClosed' => '1518789995'
+        }
+      }
+    )
+  end
+
+  describe '#issue_attributes' do
+    it 'contains the expected values' do
+      expected_attributes = {
+        title: 'Title'.ljust(255, '.'),
+        description: '# This is markdown\n it can contain more text.',
+        state: :closed,
+        created_at: Time.at(1518688921),
+        closed_at: Time.at(1518789995)
+      }
+
+      expect(task.issue_attributes).to eq(expected_attributes)
+    end
+  end
+end
diff --git a/spec/lib/gitlab/phabricator_import/worker_state_spec.rb b/spec/lib/gitlab/phabricator_import/worker_state_spec.rb
new file mode 100644
index 00000000000..a44947445c9
--- /dev/null
+++ b/spec/lib/gitlab/phabricator_import/worker_state_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe Gitlab::PhabricatorImport::WorkerState, :clean_gitlab_redis_shared_state do
+  subject(:state) { described_class.new('weird-project-id') }
+  let(:key) { 'phabricator-import/jobs/project-weird-project-id/job-count' }
+
+  describe '#add_job' do
+    it 'increments the counter for jobs' do
+      set_value(3)
+
+      expect { state.add_job }.to change { get_value }.from('3').to('4')
+    end
+  end
+
+  describe '#remove_job' do
+    it 'decrements the counter for jobs' do
+      set_value(3)
+
+      expect { state.remove_job }.to change { get_value }.from('3').to('2')
+    end
+  end
+
+  describe '#running_count' do
+    it 'reads the value' do
+      set_value(9)
+
+      expect(state.running_count).to eq(9)
+    end
+
+    it 'returns 0 when nothing was set' do
+      expect(state.running_count).to eq(0)
+    end
+  end
+
+  def set_value(value)
+    redis.with { |r| r.set(key, value) }
+  end
+
+  def get_value
+    redis.with { |r| r.get(key) }
+  end
+
+  def redis
+    Gitlab::Redis::SharedState
+  end
+end
-- 
cgit v1.2.3