Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

テスト環境でユーザーとアイコン画像が紐付けされるようにした #5096

Merged
merged 6 commits into from
Jul 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/models/company.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,7 @@ def logo_url
else
image_url('/images/companies/logos/default.png')
end
rescue ActiveStorage::FileNotFoundError
image_url('/images/companies/logos/default.png')
end
end
1 change: 1 addition & 0 deletions db/seeds.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,4 @@
]

ActiveRecord::FixtureSet.create_fixtures 'db/fixtures', tables
Bootcamp::Setup.attachment
56 changes: 56 additions & 0 deletions test/fixtures/active_storage/attachments.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# companies
company1_logo:
name: logo
record: company1 (Company)
blob: company1_logo_blob

company2_logo:
name: logo
record: company2 (Company)
blob: company2_logo_blob

company3_logo:
name: logo
record: company3 (Company)
blob: company3_logo_blob

company4_logo:
name: logo
record: company4 (Company)
blob: company4_logo_blob

# users
hatsuno_avatar:
name: avatar
record: hatsuno (User)
blob: hatsuno_avatar_blob

komagata_avatar:
name: avatar
record: komagata (User)
blob: komagata_avatar_blob

machida_avatar:
name: avatar
record: machida (User)
blob: machida_avatar_blob

mentormentaro_avatar:
name: avatar
record: mentormentaro (User)
blob: mentormentaro_avatar_blob

mineo_avatar:
name: avatar
record: mineo (User)
blob: mineo_avatar_blob

tanaka_avatar:
name: avatar
record: tanaka (User)
blob: tanaka_avatar_blob

yameo_avatar:
name: avatar
record: yameo (User)
blob: yameo_avatar_blob
26 changes: 26 additions & 0 deletions test/fixtures/active_storage/blobs.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# Pull Request #4182(https://github.com/fjordllc/bootcamp/pull/4182) で Rails 7 への移行完了後、
# ActiveStorage::FixtureSet.blob を使うように変更する

# companies
company1_logo_blob: <%= ActiveStorage::Blob.fixture filename: "companies/logos/1.jpg" %>

company2_logo_blob: <%= ActiveStorage::Blob.fixture filename: "companies/logos/2.jpg" %>

company3_logo_blob: <%= ActiveStorage::Blob.fixture filename: "companies/logos/3.jpg" %>

company4_logo_blob: <%= ActiveStorage::Blob.fixture filename: "companies/logos/4.jpg" %>

# users
hatsuno_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/hatsuno.jpg" %>

komagata_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/komagata.jpg" %>

machida_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/machida.jpg" %>

mentormentaro_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/mentormentaro.jpg" %>

mineo_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/mineo.jpg" %>

tanaka_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/tanaka.jpg" %>

yameo_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/yameo.jpg" %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

現在、test/fixture/files には companies と users 以外に、

  • articles/ogp_images/test.jpg
  • practices/ogp_images/1.jpg

がありますが、これらはセットアップ時にアタッチしなくてよい画像でしょうか?
これらの画像の用途を把握できていないのですが、test/fixture/files 以下に扱いが異なるファイルがあるのが気になったので、確認できたらいいなと思いました!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#4249 の方で2つの画像ファイルについて質問しました。
komagataさんからご回答いただき、

アップロードのテストに使っているやつかもです。そうであればアタッチする必要はありません。(その辺りのテストを確認してみてください)

とのことです。
これらの画像の用途としては、
test/system/articles_test.rb
test/system/practices_test.rb
test/system/practice/completion_test.rb
で画像のアップロードのテストに使われているだけなので、アタッチの必要はなさそうです。

11 changes: 11 additions & 0 deletions test/system/attachments_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# frozen_string_literal: true

# Pull Request #4182(https://github.com/fjordllc/bootcamp/pull/4182) で Rails 7 への移行完了後に削除する
require 'application_system_test_case'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このテストは Rails 7 へのアップデートが完了後、デフォルトの機能で fixture の画像をアタッチするようになったら不要になるかなと思うので、他の変更箇所にあるコメントと同じようにその旨を書いておくといいかもなと思いました!

Copy link
Contributor Author

@tomonariha tomonariha Jul 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かにコメントがあった方がいいですね!
Rails7移行後のためのコメントを記載するようにしました。
2512d0d

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントの対応ありがとうございます!

他の変更箇所にあるコメントと同じようにその旨を書いておく

僕のこの書き方が悪くて申し訳ないのですが...🙇‍♂️

ActiveStorage::FixtureSet.blob を使うように変更する というコメントではなく、テスト自体(このファイルごと)不要になることを書いた方が適切かなと思いました。
現在は Rails 7 から使えるようになる機能を自作で書いているので、テストで確認するのは良いと思うのですが、Rails 7 へアップデートで ActiveStorage::FixtureSet.blob を使うようになると、gem の機能をわざわざテストすることになるので不要になるといった考えです(何か勘違いしていたらすみません...)

テストのプラクティスの説明にあった以下の内容のケースに当てはまるかなと思ってます。

modelのvalidationなど、railsの機能、gemの機能をテストしてしまう。(gemのテストはgemの中でそれぞれやってるので必要ない)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rails7移行後でもアタッチのテストは必要かな?と思っていましたが、確かにActiveStorage::FixtureSet.blobでアタッチすればrailsの機能ということでテストはいらないですね。
理解が悪くて申し訳ないです🙇‍♂️
テストを削除するよう記述を変更しました。8512b43

class AttachmentsTest < ApplicationSystemTestCase
test 'attachment user avatar' do
visit_with_auth "/users/#{users(:komagata).id}", 'komagata'
assert find('img.user-profile__user-icon-image')['src'].include?('komagata.png')
end
end
19 changes: 19 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,22 @@ class ActionDispatch::IntegrationTest
ActiveSupport.on_load(:action_dispatch_system_test_case) do
ActionDispatch::SystemTesting::Server.silence_puma = true
end

# Rails 7 の ActiveStorage::FixtureSet.blob と同様の機能を実装
# Pull Request #4182(https://github.com/fjordllc/bootcamp/pull/4182) でRails 7 への移行完了後に削除する
# => test/fixtures/active_storage/blobs.yml でActiveStorage::FixtureSet.blob を使うように変更する
module BlobFixtureSet
def fixture(filename:, **attributes)
blob = new(
filename: filename,
key: generate_unique_secure_token
)
io = Rails.root.join("test/fixtures/files/#{filename}").open
blob.unfurl(io)
blob.assign_attributes(attributes)
blob.upload_without_unfurling(io)

blob.attributes.transform_values { |values| values.is_a?(Hash) ? values.to_json : values }.compact.to_json
end
end
ActiveStorage::Blob.extend BlobFixtureSet