Skip to content
Open
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 actionpack/lib/action_dispatch/http/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,8 @@ def body_stream #:nodoc:

# TODO This should be broken apart into AD::Request::Session and probably
# be included by the session middleware.
# もしかしたらコメント削除してしまっていいのかも?
# [Just reading flash messages should not create a session if one does n… · rails/rails@a12b76b](https://github.com/rails/rails/commit/a12b76b09e98493c1e4aee147416ae5999402298)

Choose a reason for hiding this comment

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

def reset_session
if session && session.respond_to?(:destroy)
session.destroy
Expand Down
2 changes: 2 additions & 0 deletions actionpack/test/controller/force_ssl_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -230,13 +230,15 @@ def test_cheeseburger_redirects_to_https
assert_equal "http://test.host/force_ssl_flash/cheeseburger", redirect_to_url

# FIXME: AC::TestCase#build_request_uri doesn't build a new uri if PATH_INFO exists
# AC::TestCase#build_request_uri がないので一回消して通るか確かめてみたい
@request.env.delete('PATH_INFO')

get :cheeseburger
assert_response 301
assert_equal "https://test.host/force_ssl_flash/cheeseburger", redirect_to_url

# FIXME: AC::TestCase#build_request_uri doesn't build a new uri if PATH_INFO exists
# AC::TestCase#build_request_uri がないので一回消して通るか確かめてみたい
@request.env.delete('PATH_INFO')

get :use_flash
Expand Down
2 changes: 2 additions & 0 deletions actionview/test/activerecord/polymorphic_routes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ def test_string
with_test_routes do
# FIXME: why are these different? Symbol case passes through to
# `polymorphic_url`, but the String case doesn't.
# これ単純に消しても問題ないコメントなのかも?

Choose a reason for hiding this comment

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

# [test for inconsistency between String and Symbol url_for handling · rails/rails@37d4415](https://github.com/rails/rails/commit/37d4415a7b433fcb987b1c6a5b51bf2d8efc5d5e)
assert_equal "http://example.com/projects", polymorphic_url("projects")
assert_equal "projects", url_for("projects")
end
Expand Down
1 change: 1 addition & 0 deletions activerecord/test/cases/base_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -887,6 +887,7 @@ def test_bignum
end

# TODO: extend defaults tests to other databases!
# これ mysql 版いけるのでは?
if current_adapter?(:PostgreSQLAdapter)
def test_default
with_timezone_config default: :local do
Expand Down
3 changes: 3 additions & 0 deletions activerecord/test/cases/finder_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ def test_second_to_last

#test with limit
# assert_equal nil, Topic.limit(1).second # TODO: currently failing
# 今日の master コンソール上で試したら nil 帰ってきたのでもう動くのではという気がする
Copy link
Owner Author

Choose a reason for hiding this comment

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

これ嘘でした><2番目のレコード返ってきちゃいますね…

assert_equal nil, Topic.limit(1).second_to_last
end

Expand Down Expand Up @@ -527,8 +528,10 @@ def test_third_to_last

# test with limit
# assert_equal nil, Topic.limit(1).third # TODO: currently failing
# 今日の master コンソール上で試したら nil 帰ってきたのでもう動くのではという気がする
assert_equal nil, Topic.limit(1).third_to_last
# assert_equal nil, Topic.limit(2).third # TODO: currently failing
# 今日の master コンソール上で試したら nil 帰ってきたのでもう動くのではという気がする
assert_equal nil, Topic.limit(2).third_to_last
end

Expand Down
4 changes: 3 additions & 1 deletion activerecord/test/cases/helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
require 'support/connection'

# TODO: Move all these random hacks into the ARTest namespace and into the support/ dir

# ロジック移動させるだけで済むのかもしれない
Thread.abort_on_exception = true

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

@willnet これ見てるんですが、移動するっていうのは、単に mv cases/helpers.rb support/helper.rb しろっていうより、ここにランダムで書いちゃってるのをsupport/以下に整理しろって意味なんでしょうか?
もし後者ならきっついなーと思いつつ、前者なら、移動と影響箇所(200くらいrequireしてましたが...)の修正でおわるかなと。

Copy link
Owner Author

Choose a reason for hiding this comment

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

@takayukishmz "these random hacks" がどこまで指すのかはわからないです(全部だとつらいですね><)。

内容てきにはsupport/config.rb support/connection.rb などに ARTest module が定義されているので、ARTest のメソッドとして定義して実行したいってことなのかなと思いました。

例えばこんな感じ

module ARTest
  def self.setup
    Thread.abort_on_exception = true
    ActiveSupport::Deprecation.debug = true
  end
end


# Show backtraces for deprecated behavior for quicker cleanup.
Expand Down Expand Up @@ -202,3 +202,5 @@ def in_time_zone(zone)
end

require 'mocha/setup' # FIXME: stop using mocha
# 根気があれば直せるのでは
# ひたすら minitest のモックの記法に置換していくだけでいける?
Copy link
Owner Author

Choose a reason for hiding this comment

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

この行削ったら落ちたテストの数

  • sqlite 75
  • mysql 94
  • postgres 87

多分重複してるのもあるけどなかなか厳しい数字

2 changes: 2 additions & 0 deletions activerecord/test/cases/validations/i18n_validation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ def replied_topic
# TODO Add :on case, but below doesn't work, because then the validation isn't run for some reason
# even when using .save instead .valid?
# [ "given on condition", {on: :save}, {}]
# とりあえず、ある程度泥臭く書けば on はサポートできそう
# そこから綺麗に書ける方法を見つけられればマージされるのでは?

Choose a reason for hiding this comment

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

これ俺やってみます。

]

COMMON_CASES.each do |name, validation_options, generate_message_options|
Expand Down