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

Flakyなテストを2つ修正した #7997

Merged
merged 6 commits into from
Aug 16, 2024
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
5 changes: 5 additions & 0 deletions config/storage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ google:
bucket: <%= ENV["GCS_BUCKET"] %>
public: true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

下記リンクを参考にfixtures用の保存先を作成しています。
Active Storage の概要 - Railsガイド

test_fixtures:
service: Disk
root: <%= Rails.root.join("tmp/storage_fixtures") %>
public: true

# Use rails credentials:edit to set the AWS secrets (as aws:access_key_id|secret_access_key)
# amazon:
# service: S3
Expand Down
28 changes: 14 additions & 14 deletions test/fixtures/active_storage/blobs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,31 @@
# ActiveStorage::FixtureSet.blob を使うように変更する

Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちらを参考に、各フィクスチャの保存先を設定しています。
ちなみに、BootcamoアプリはRails6.1であり、RailsガイドにあるようなActiveStorage::FixtureSet.blobという機能はRails7からの機能なのでありませんが、こちらのPRのtest/system/test_helper.rbによってActiveStorage::Blob.fixtureとすることで同等の機能が使用できるようになっています。

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

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

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

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

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

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

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

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

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

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

yameo_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/yameo.jpg" %>
yameo_avatar_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/yameo.jpg", service_name: "test_fixtures" %>

komagata_profile_image_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/komagata.jpg" %>
komagata_profile_image_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/komagata.jpg", service_name: "test_fixtures" %>

machida_profile_image_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/machida.jpg" %>
machida_profile_image_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/machida.jpg", service_name: "test_fixtures" %>

kimuramitai_profile_image_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/default.jpg" %>
kimuramitai_profile_image_blob: <%= ActiveStorage::Blob.fixture filename: "users/avatars/default.jpg", service_name: "test_fixtures" %>
1 change: 1 addition & 0 deletions test/system/users_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ class UsersTest < ApplicationSystemTestCase
visit_with_auth '/users', 'komagata'
assert_selector '.users-item', count: 24
fill_in 'js-user-search-input', with: '木村です'
find('#js-user-search-input').send_keys :return
assert_text 'Kimura Tadasi', count: 1
end

Expand Down
16 changes: 16 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,22 @@ class ActiveSupport::TestCase
teardown do
ActiveStorage::Current.host = nil
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちらを参考にして、並列のプロセスごとに、保存先にそれぞれ番号がふられるように設定しています。

Railsガイドにある書き方はRails7から入った機能です。
Rails6.1で同様のことをするためinstance_variable_setを使用して修正しています。
そしてBootcampアプリがRails7になったときのためにコメントを記載しています。
参考:Parallelizing setup for ActiveStorage doesnt expose setter · Issue #43616 · rails/rails
(ちなみにRails6.1のRailsガイドには並列実行時のテスト用保存先の別名の付け方の記載はありません)

# Rails7になったら以下のように修正する
# parallelize_setup do |i|
# ActiveStorage::Blob.service.root = "#{ActiveStorage::Blob.service.root}/storage-#{i}"
# ActiveStorage::Blob.services.fetch(:test_fixtures).root = "#{ActiveStorage::Blob.services.fetch(:test_fixtures).root}/fixtures-#{i}"
# end
# 参考: https://guides.rubyonrails.org/active_storage_overview.html#discarding-files-created-during-tests
parallelize_setup do |i|
ActiveStorage::Blob.service.instance_variable_set(:@root, "#{ActiveStorage::Blob.service.root}/storage-#{i}")
ActiveStorage::Blob.services.fetch(:test_fixtures).instance_variable_set(:@root, "#{ActiveStorage::Blob.services.fetch(:test_fixtures).root}/fixtures-#{i}")
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

アップロード用保存先フォルダに関しては、Railsガイドにはafter_teardownを使用し各テストごとに削除するよう記載してありますが、そのように記載しCIを実行するとActiveStorage::FileNotFoundError: ActiveStorage::FileNotFoundErrorが発生する回数が増えました。
なぜエラーが増えるのか調査しましたが、理由はわかりませんでした。
そのため、テストが全て終了した後にデータをクリーンアップするように設定しています。

Minitest.after_run do
FileUtils.rm_rf(ActiveStorage::Blob.service.root)
FileUtils.rm_rf(ActiveStorage::Blob.services.fetch(:test_fixtures).root)
end
end

class ActionDispatch::IntegrationTest
Expand Down