注意点

このガイドの目的は、GitLab CEとEEの開発者が遭遇する可能性のある、あるいは避けるべき「ゴッチャ」を文書化することです。

app/assets ディレクトリからファイルを読み込まないでください。

GitLab 10.8 以降では、Omnibus はアセットコンパイル後に app/assets ディレクトリ を削除しました。ee/app/assets,vendor/assets ディレクトリも同様に削除されます。

このため、OmnibusがインストールされたGitLabインスタンスでは、このディレクトリからのファイルの読み込みに失敗します：

file = Rails.root.join('app/assets/images/logo.svg')

# This file does not exist, read will fail with:
# Errno::ENOENT: No such file or directory @ rb_sysopen
File.read(file)

シーケンスで生成された属性の絶対値に対するアサートは行わないようにしましょう。

次のファクトリーを考えてみましょう：

FactoryBot.define do
  factory :label do
    sequence(:title) { |n| "label#{n}" }
  end
end

以下のAPI仕様を考えてみましょう：

require 'spec_helper'

RSpec.describe API::Labels do
  it 'creates a first label' do
    create(:label)

    get api("/projects/#{project.id}/labels", user)

    expect(response).to have_gitlab_http_status(:ok)
    expect(json_response.first['name']).to eq('label1')
  end

  it 'creates a second label' do
    create(:label)

    get api("/projects/#{project.id}/labels", user)

    expect(response).to have_gitlab_http_status(:ok)
    expect(json_response.first['name']).to eq('label1')
  end
end

この仕様を実行すると、私たちが期待するようなことは行われません：

1) API::API reproduce sequence issue creates a second label
   Failure/Error: expect(json_response.first['name']).to eq('label1')

     expected: "label1"
          got: "label2"

     (compared using ==)

これは、FactoryBotのシーケンスが例ごとにリセットされないためです。

シーケンスが生成する値は、ファクトリーを使用する際に一意性制約を持つ属性を明示的に設定する必要がないようにするためだけに存在することを覚えておいてください。

解答

シーケンスで生成された属性の値に対してアサートする場合は、明示的に設定する必要があります。また、設定する値はシーケンスパターンにマッチしないようにします。

たとえば:label ファクトリを使用する場合、create(:label, title: 'foo') と書くのはよいのですがcreate(:label, title: 'label1') と書くのはよくありません。

以下は固定API仕様です：

require 'spec_helper'

RSpec.describe API::Labels do
  it 'creates a first label' do
    create(:label, title: 'foo')

    get api("/projects/#{project.id}/labels", user)

    expect(response).to have_gitlab_http_status(:ok)
    expect(json_response.first['name']).to eq('foo')
  end

  it 'creates a second label' do
    create(:label, title: 'bar')

    get api("/projects/#{project.id}/labels", user)

    expect(response).to have_gitlab_http_status(:ok)
    expect(json_response.first['name']).to eq('bar')
  end
end

RSpecではexpect_any_instance_of やallow_any_instance_of の使用は避けてください。

なぜ

  • 孤立しているわけではないからです。

  • スタブしたいメソッドがプリペンドモジュールで定義されている場合、このメソッドは動作しません。このようなエラーが発生します：

     1.1) Failure/Error: expect_any_instance_of(ApplicationSetting).to receive_messages(messages)
      Using `any_instance` to stub a method (elasticsearch_indexing) that has been defined on a prepended module (EE::ApplicationSetting) is not supported.

代替案expect_next_instance_of allow_next_instance_of,expect_next_found_instance_of またはallow_next_found_instance_of

と書く代わりに

# Don't do this:
expect_any_instance_of(Project).to receive(:add_import_job)

# Don't do this:
allow_any_instance_of(Project).to receive(:add_import_job)

私たちは書くことができます：

# Do this:
expect_next_instance_of(Project) do |project|
  expect(project).to receive(:add_import_job)
end

# Do this:
allow_next_instance_of(Project) do |project|
  allow(project).to receive(:add_import_job)
end

# Do this:
expect_next_found_instance_of(Project) do |project|
  expect(project).to receive(:add_import_job)
end

# Do this:
allow_next_found_instance_of(Project) do |project|
  allow(project).to receive(:add_import_job)
end

Active Recordはモデルクラスの.new メソッドを呼び出してオブジェクトをインスタンス化しているわけではないので、expect_next_found_instance_of またはallow_next_found_instance_of モックヘルパーを使用して、Active Recordのクエリメソッドとファインダメソッドから返されるオブジェクトのモックをセットアップする必要があります。

また、expect_next_found_(number)_instances_of とallow_next_found_(number)_instances_of ヘルパーを使用することで、同じ Active Record モデルの複数のインスタンスにモックと期待値を設定することができます；

expect_next_found_2_instances_of(Project) do |project|
  expect(project).to receive(:add_import_job)
end

allow_next_found_2_instances_of(Project) do |project|
  allow(project).to receive(:add_import_job)
end

インスタンスを特定の引数で初期化したい場合は、次のように渡すこともできます：

# Do this:
expect_next_instance_of(MergeRequests::RefreshService, project, user) do |refresh_service|
  expect(refresh_service).to receive(:execute).with(oldrev, newrev, ref)
end

この場合、次のようになります：

# Above expects:
refresh_service = MergeRequests::RefreshService.new(project, user)
refresh_service.execute(oldrev, newrev, ref)

To-Do notrescue Exception

“Ruby でrescue Exception => e をするのはなぜスタイルが悪いのか?” を参照してください。

このルールはRuboCop._によって自動的に適用されます。

ビューではインライン JavaScript を使用しないでください。

インライン:javascript Haml フィルタを使用すると、パフォーマンスのオーバーヘッドが発生します。インライン JavaScript の使用はコードを構成する良い方法ではないので避けるべきです。

イニシャライザでこれら2つのフィルタを削除しました。

さらに読む

プリコンパイルを必要としないアセットの保存

ユーザーに提供する必要のあるアセットは、app/assets ディレクトリに保存され、後でプリコンパイルされ、public/ ディレクトリに置かれます。

しかし、app/assets 内にあるどのファイルの内容にも、アプリケーションコードからアクセスすることはできません。これは、スペースを節約するために、本番インストールではこのフォルダを含めないようにしているからです。

support_bot = Users::Internal.support_bot

# accessing a file from the `app/assets` folder
support_bot.avatar = Rails.root.join('app', 'assets', 'images', 'bot_avatars', 'support_bot.png').open

support_bot.save!

上記のコードは内部環境では動作しますが、本番インストールではapp/assets フォルダーが含まれないためエラーになります。

解答

別の方法として、lib/assets フォルダがあります。以下の条件を満たすアセット（画像など）をリポジトリに追加する必要がある場合に使用してください：

  • アセットを直接ユーザーに提供する必要がない（したがって、コンパイル済みである必要はない）場合。
  • アセットにはアプリケーション・コードからアクセスする必要があります。

要するに

プリコンパイルしてエンドユーザーに提供する必要のあるアセットの保存にはapp/assets を使用してください。エンドユーザーに直接提供する必要はないが、アプリケーションコードからのアクセスが必要なアセットの保存にはlib/assets を使用してください。

参考用 MR:!37671