4
3

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 3 years have passed since last update.

OSSに貢献してリリースノートに名前が載った話

Last updated at Posted at 2020-08-11

概要

このたび、OSS(Gradle)に貢献してリリースノートに名前が載りました。めっちゃうれしいです!
といっても、実はあまり胸を張れるような話ではないです。修正内容は地味ですし、詳細は後述しますが色々やらかしてだいぶご迷惑をおかけしてしまいました…
そんな体たらくなのに、一応コントリビュータとして扱ってくれるGradle開発元の皆様の懐の深さには感謝しかありません。

まあ、形はどうあれ私自身にとっては貴重な経験だったので、きっかけからマージされるまでの経過について記録を残すということで投稿します。

なお、該当のリリースノートはこちらです。
https://docs.gradle.org/6.6/release-notes.html

何をしたか

JUnit5で書かれたテストをGradleで実行した際に出力されるテスト結果のXMLの内容をちょっといじりました。具体的には @DisplayName アノテーションが付与されている場合はその値がXMLに出力されるようにしました。
@DisplayName とは、テストクラスやテストメソッドについて、テストの実行結果に表示される名前を設定する、JUnit5で導入されたアノテーションです。こんな感じで使います。

@DisplayName("計算クラスのテスト")
class CalcTest {
    @Test
    @DisplayName("addメソッドは2と3を渡すと5を返す")
    void testAdd() {
        var calc = new Calc();
        assertEquals(5, calc.add(2, 3));
    }
}
Screen Shot 2020-07-25 at 20.35.44.png

JUnit4まではそういった機能がなかったので、テストの実行結果ではテストメソッド名がテストの名前として出力されていました。テストの名前をわかりやすくするために、テストメソッド名を日本語で書く人もいたりしました。
@DisplayName を使えばテストメソッド名とは別にテストの名前を設定でき、テストの実行結果ではメソッド名ではなく @DisplayName で指定した名前が表示されるわけです。これにより、メソッド名を日本語で書くという、人によっては抵抗がある行為をしなくて済みます。

……と言いたいところなのですが、テスト結果を出力するツールが @DisplayName に対応していない場合があり、その場合は @DisplayName で設定した値は無視されてメソッド名が表示されてしまいます。
Gradleも @DisplayName に完全には対応しておらず1、テスト結果のXMLファイルについては @DisplayName の値を無視してテストのクラス名やメソッド名が出力される実装となっていたため、今回の修正に至ったわけです。

経過

きっかけ

(けっこう前の話なので記憶が曖昧な部分もあり、細部が実際と違うかもしれません)

うちのチームでは長らくJUnit4を使っていたのですが、JUnit5への移行を提案したところあっさり認められました。ただ、そのためには当然チームのみんながJUnit5を使えるようになる必要があるということで、チーム内で簡単にJUnit5勉強会を開催しました。
勉強会の中で件の @DisplayName についても言及しまして、 @DisplayName はできるだけ設定しましょうと偉そうに述べました。
そのすぐ後だったと思うのですが、チームのメンバーの一人から、Jenkinsのテスト結果の表示画面に @DisplayName の設定値が反映されないと指摘を受けました。確認してみると、たしかにその通りでした。 @DisplayName を使おうと呼び掛けた直後にこれだったので、少々ばつが悪い感じになりました…

原因調査

Jenkinsはテスト結果のXMLファイルを読み取ってテスト結果画面を表示するのですが、そのことはもともと知っていたので、まずそのXMLファイルの中身を覗いてみました。結果、XMLには @DisplayName の値ではなくクラス名やメソッド名が出力されていました。原因はJenkinsではなく、そのXMLファイルを生成したツールにありそうです。
どうやって突き止めたのか覚えてないですが、そのXMLを生成しているのはGradleであるとわかりました。つまりGradleが @DisplayName に対応していないことが原因でした。

なんとなくcloneしてみる

@DisplayName の値が無視されたところで大した影響はないのですが、なんとなく興味が湧いたのでGradleのソースをcloneしてみました。
XMLの要素名とか属性名で検索してみたところ、一箇所のみがヒットしました。つまりそこがまさにXMLの出力処理を行うコードでした。ざっと読んでみて、この程度の修正だったら俺でもできるんじゃね?と思い、挑戦してみることにしました。
以前、非常に軽微なドキュメント修正だけですがJenkinsに貢献したことがあったので、精神的なハードルは多少下がっていたのかもしれません。
OSS Gate東京ワークショップ2019-12-14に参加してJenkinsにコントリビュートした話

貢献方法を調べる

OSSかどうか調べる

OSSに貢献するには、まずはその対象が本当にOSSなのかどうかを確かめる必要があります。OSSじゃなかったら貢献もなにもないですからね…
OSSかどうかはライセンスを調べることでわかります。上の記事にも書きましたが(というか上の記事を見ながらこれを書いてますが)、ライセンスが下記の一覧の中にあればOSSだと言えるそうです。
https://opensource.org/licenses/alphabetical
そして、ライセンスは一般的には公式サイトかGitHubに書かれています。

GradleのライセンスはApache License 2.0ですね。
https://github.com/gradle/gradle/blob/master/LICENSE

Apache License 2.0はバッチリ一覧にありますので、GradleはOSSであると言えます。

Gradleへの貢献方法調査〜最初のプルリク

OSSに貢献する方法は、そのOSSごとに異なります。大抵は貢献方法が書かれたドキュメントがあるので、それを探します。
Gradleについては下記のようなドキュメントがありましたので、それに従って作業を進めました。
https://github.com/gradle/gradle/blob/master/CONTRIBUTING.md

当然全て英語なので、読むのにけっこう時間がかかりました…
通常はまずIssueを開くのですが、本件のIssueは他の方が既に開いていました。
https://github.com/gradle/gradle/issues/11445

それならこれに対する修正という形でプルリクを投げればいいだろうと思いました。修正は簡単で修正量も少ないように思えたので、いきなりプルリクを投げても大丈夫だろうと思いました。Jenkinsの時もそうだったし。
(まあ、その認識は甘過ぎたということを後ほど思い知ることになるのですが…)

修正を施し、UTを追加し、動作確認もして、翻訳ツールに頼りながら辿々しい英文を書き、ドキドキしながら最初のプルリクを投げました。

反応あり

プルリクを投げてから少し間が空きましたが、反応がありました。曰く、修正の影響でテストが失敗しているとのこと…
修正したソースが含まれるプロジェクトのテストについては成功することを確認していたのですが、別のプロジェクトのテストが失敗しているということでした。全く気づいてませんでした…大きな反省点の一つです。
私が行った修正では @DisplayName が付与されていない場合の結果が修正前と変わってしまっていました。具体的には、修正前はクラス名がFQNだったのですが、修正後はパッケージ名を含まないクラス名となってしまっていました。動作確認は行ったのですが、デフォルトパッケージのクラスで確認したため、修正前後で差分がなくて気付きませんでした。大変お恥ずかしい話です…

再修正

指摘を受けて、修正方法を検討しました。修正するには @DisplayName が付与されているかどうかを判定する必要があります。それくらい大したことないだろうと最初は思っていたのですが、実際にはけっこう大変だとわかりました。
どこかに @DisplayName の値を取得しているコードがあるはずなんですが想像以上に範囲が広くて、そのコードがどこにあるのか特定するのにめちゃくちゃ時間がかかりました。また、該当コードから最初に修正した箇所まで値を受け渡していく必要がありますが、その過程でけっこうな数のファイルを修正することになり、その影響でUTもあちこちで失敗して、なかなか苦労しました。それ以外にも様々な種類の実行時エラーを踏み抜きました。思った以上に大変な問題に首を突っ込んでしまったらしいと、プルリクを投げたことを若干後悔していましたが、最終的にはなんとか動くようになりました。

プルリクを投げた後に再修正をした場合、どういう操作を行えばいいのかわからなかったのですが、ググったらそれらしい情報がトップに出てきたので助かりました。
https://teratail.com/questions/122843

プルリクを修正したら、すぐに反応がありました。一部指摘を受けましたが、修正の方向性は間違ってないみたいな主旨のコメント(たぶん)をもらってうれしかったです。
とりあえず、指摘の箇所は単純に削除すればよさそうだったので、そのようにしました。

再レビュー依頼をかける

上記の修正後は非常に素早くレビューしてもらえたのですが、今回はなかなかレビューのコメントをもらえませんでした。なんでだろうと思っていたのですが、どうやら私がレビュー依頼をしていなかったのが理由だったようです…
修正したという主旨のコメントは書いていたのでそれでレビュー依頼をした気になっていたのですが、どうやらGitHubの仕組みに則ってレビュー依頼を出さないといけないようでした。
レビュアーの名前の右にある点をクリックするとレビュー依頼が出せるみたいなので、そうしました。

レビュアーによる修正〜マージへ

レビュー依頼をかけた後はわりとすぐに反応をもらえたのですが、私が投げたプルリクについてはクローズされてしまい、新たなプルリクが作られて、レビュアーが私の修正したコードをガッツリ再修正した後にマージされました。
レビュアーが直々にレビュイーのコードを修正するって一般的なことなんでしょうか?まあ、普通に考えれば少なくとも喜ばしいことではないですけど。コイツには任せておけんみたいな感じだったのかなあ…
修正されるということはつまり問題があるということなので、私はまだまだ実力が足りないのだなあと思いました。

まあ、修正されちゃったとはいえ一応私が書いたコードがマージされたというのは非常にうれしかったです。
また、一時はプルリクを投げたことをちょっと後悔していたくらいですから、なんとか最後まで終わったということで、肩の荷が下りた、解放されたという思いが強かったですw

反省点まとめ

  • 修正後の動作確認が不十分だった
    • その結果として修正難易度を見誤った(そのおかげで貢献できたとも言えますが…w)
  • テストが失敗しているのに気づかなかった
  • レビュー依頼をせずにレビュアーを無駄に待たせてしまった(仕組みを知らなかった。調査不足)
  • 上には書いてなかったが、よく考えたら修正用ブランチを作ってなかった

あと反省点ではないですが、力不足感が半端じゃないので修正差分を見て勉強させてもらおうと思います。(早くやれという感じですが…)

終わりに

本当は「OSSへの貢献は難しくないよ!みんなもやろう!」みたいな記事を書きたかったところですが、実際やってみたらけっこう難しかったです…(実力不足のせいでもありますし、そもそもピンキリだから簡単な場合もあるでしょうけど)

とりあえず、今回「OSSへの貢献」という経験を積むことができたので、今後また同様のことがあった時に多少は自信を持って取り組めるかなと思いました。修正されちゃったけど!

今後は、積極的に貢献のネタを探し回ったりはしないと思いますが、普段使っているOSSで何か問題を見つけたら、自分で直せる程度の問題なのかどうか確認するくらいのことはしようかなと思います。

おまけ

色々とお恥ずかしい内容ですが、該当のプルリクを晒しておきます。
https://github.com/gradle/gradle/pull/12541

  1. テスト結果を表示するHTMLファイルについては、私が手を付ける前から対応されてました。

4
3
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
4
3

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?