Exceptions can be hard to catch in test environments
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 35/100
調査の方向性
lib/temporal/workflow.rb の StandardError の rescue 付近から始め、Temporal::Testing.local! がどのように workflow を実行するかを追跡します。キーワードが欠落している例を再現し、そのうえで、通常の workflow の処理を変更せずに workflow の例外が表面化するローカルテストの挙動を特定して文書化します。テストが有用な stack trace 付きで例外を明らかにすれば完了です。
索引モデルが issue の本文から書いたものです。
説明
Cool library, and thank you for your hard work on it! I'm trying to help improve it through this feedback, and would be willing to open a PR if there's guidance provided on desirable compromises or solutions.
The way that Temporal::Workflow rescues StandardError makes catching exceptions in tests hard.
Take this spec as an example:
$global_var = 0
class HelloWorldWorkflow < Temporal::Workflow
def execute(arg1, required_key:)
$global_var += 1
nil
end
end
describe HelloWorldWorkflow do
it "doesn't allow exceptions to surface very easily" do
Temporal::Testing.local! do
Temporal.start_workflow(HelloWorldWorkflow, "foo", required_key: "bar")
expect($global_var).to eq(1)
Temporal.start_workflow(HelloWorldWorkflow, "foo") # incorrect arguments
expect($global_var).to eq(2)
end
end
end
The result is:
expected: 2
got: 1
(compared using ==)
Yes, I can see in the logs that an exception is logged, but I feel like in tests raising exceptions should be the rule. Any number of things can break downstream within a workflow, and sometimes (when not directly testing the unit that is the workflow) it's useful to call a thing, and not have to make an assertion that it didn't log an exception -- if that makes sense. Rescuing StandardError is heavy handed in the tests, because allowing those to raise is way more useful in identifying any issues.
To address this I've wrapped the base class with my own, so I can re-raise the exception in a way that Temporal doesn't try to handle. I'm wondering if Temporal::Testing should take this into consideration and not rescue any exceptions when executing the workflow locally in tests.
class ApplicationWorkflow < Temporal::Workflow
def execute(*args, **kwargs)
perform(*args, **kwargs)
rescue => e
raise(Exception, e.message) if defined?(Temporal::Testing) && Temporal::Testing.local?
raise(e)
end
end
class HelloWorldWorkflow < ApplicationWorkflow
def perform(arg1, required_key:)
$global_var += 1
nil
end
end
And the more useful result now includes the exception and stops the execution of the spec, which is what would be most helpful.
Exception: missing keyword: :required_key
It's still not the best solution though because it now has two rescues and pollutes the stack trace.
- 主要言語
- Ruby
- スター
- 288
- フォーク
- 113
- 平均マージ
- 10日 15時間
- マージ済み PR(30日)
- 2
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
coinbase/temporal-ruby のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 82/100
coinbase/temporal-ruby#361 ·
-
難易度 3/5 1〜2日 初心者へのやさしさ 35/100
coinbase/temporal-ruby#341 ·
-
難易度 5/5 1週間以上 初心者へのやさしさ 25/100
coinbase/temporal-ruby#328 · コメント 1 件 ·
-
難易度 3/5 1〜2日 初心者へのやさしさ 45/100
coinbase/temporal-ruby#326 · コメント 1 件 ·
-
難易度 4/5 3〜5日 初心者へのやさしさ 25/100
coinbase/temporal-ruby#324 · コメント 3 件 · リアクション 2 件 ·
coinbase/temporal-ruby の issue をすべて見る
似ている issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 65/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
googleapis/google-api-ruby-client#28001 · リアクション 5 件 ·
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100