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

add a test #20

Merged
merged 4 commits into from
Aug 12, 2022
Merged

add a test #20

merged 4 commits into from
Aug 12, 2022

Conversation

fumieval
Copy link
Contributor

@fumieval fumieval commented Aug 8, 2022

StdoutTransportのテストを作った

@fumieval fumieval requested review from ruicc and ccycle August 8, 2022 07:31
tests/stdout.hs Outdated
logM [ #emerg, "lorem ipsum" ]


threadDelay $ 1000 * 10
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ダメじゃんこれってなった

@ccycle
Copy link
Contributor

ccycle commented Aug 9, 2022

テストケースを足してみました。

@fumieval
Copy link
Contributor Author

fumieval commented Aug 9, 2022

お、ありがとうございます。せっかく増やしてもらったところ恐縮ですが、一連の処理は同じ関数として定義して、ロガーのレベルを引数として渡すようにしてもらえるとコードの重複が減り、テストも一つにまとめられるので、そうしてもらえるとありがたいです

@ccycle
Copy link
Contributor

ccycle commented Aug 9, 2022

コメントありがとうございます。修正してひとつにまとめてみましたが、どうでしょうか?

@fumieval
Copy link
Contributor Author

fumieval commented Aug 9, 2022

ありがとうございます。「ロガーのレベルを引数として渡す」ところはまだできていないようですね

@ccycle
Copy link
Contributor

ccycle commented Aug 9, 2022

すみません、「ロガーのレベルを引数として渡す」の部分を反映していませんでした。一応やってみたんですがこんな感じでしょうか?

@fumieval
Copy link
Contributor Author

fumieval commented Aug 9, 2022

まさにそういうことです!

Copy link
Contributor

@ccycle ccycle left a comment

Choose a reason for hiding this comment

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

手元の環境ではcabal run test:stdout でテストが通るので、自分は大丈夫だと思います。

Copy link
Contributor

@ruicc ruicc left a comment

Choose a reason for hiding this comment

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

LGTM!

@fumieval fumieval merged commit 210dc33 into master Aug 12, 2022
@ccycle ccycle deleted the test branch September 8, 2022 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants