Fix bugs in Process.checkRun
of CartonDriver
#463
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Process.checkRun
が実装されていますが、少し変です。forwardExit
フラグが指定されている場合に、terminationHandler
経由でexit
していますが、それとは別に
waitUntilExit()
の後にもexit
するフローがあります。これだと
exit
する経路が2つあって、どちらを通るかは並行処理次第であり不安定に見えます。
そもそも
waitUntilExit()
している関数なので、そこから復帰した後に
exit
へ入れば良いので、terminationHandler
を使わないシンプルな形に書き換えます。既存のコードで
waitUntilExit
の後に書いてあるexit
は、ステータスコードが失敗なら突入するようになっているので、
forwardExit
の機能としては壊れています。よってこれは
forward
しない場合の機能のようです。実際にこちらが使われるのは、
carton test
に2つあり、plugin に対して
internal-get-build-command
を呼ぶところと、受け取ったそのビルドコマンドを実行するところです。
この2箇所については、
もしコマンドが失敗した時はステータスコードを転送して即死するのではなく、
制御ロジックとして例外を投げる方が望ましいと思います。
その方が失敗した時に状況が理解しやすいです。
これらのコマンドは driver が変身してるわけではないからです。
出力例です。
test経由でビルドが始まったときに C-c を押して殺しています。
例外のメッセージを使って、
死んだコマンドのコマンドラインがコンソールに出せるので、
トラブルシュートしやすくなっていると思います。