Updating tests per #43 on GitHub re: new ggplot2 testing#45
Updating tests per #43 on GitHub re: new ggplot2 testing#45rossdrucker merged 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe test files for various sports plotting functions were updated to use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/testthat/test-plots-and-features-curling.R (1)
6-7: Preferexpect_s3_class()for brevity & richer failure messages
ggplot2::is_ggplot()works, buttestthat::expect_s3_class(wcf_sheet, "ggplot")makes the intent explicit, removes the extra helper call, and yields clearer diagnostics if the check fails. Consider switching across the suite for consistency with commontestthatidioms.tests/testthat/test-plots-and-features-volleyball.R (1)
6-7: Considerexpect_s3_class()for concise, idiomatic testingSame rationale as in the curling tests:
expect_s3_class(ncaa_court, "ggplot")keeps the assertion terse and leverages
testthat-native helpers.tests/testthat/test-plots-and-features-lacrosse.R (1)
6-7: Swap toexpect_s3_class()for alignment with testthat styleApplying
expect_s3_class(nll_field, "ggplot")shortens the assertion and unifies style with other object-type checks you may have.tests/testthat/test-plots-and-features-tennis.R (1)
6-7: Minor style tweak: useexpect_s3_class()For consistency with common
testthatpractice and the rest of the suite:expect_s3_class(itf_court, "ggplot")tests/testthat/test-plots-and-features-hockey.R (1)
6-7: Useexpect_s3_class()for clearer intentReplacing with
expect_s3_class(nhl_rink, "ggplot")trims boilerplate and standardises assertions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
tests/testthat/test-plots-and-features-baseball.R(3 hunks)tests/testthat/test-plots-and-features-basketball.R(3 hunks)tests/testthat/test-plots-and-features-curling.R(3 hunks)tests/testthat/test-plots-and-features-football.R(3 hunks)tests/testthat/test-plots-and-features-hockey.R(3 hunks)tests/testthat/test-plots-and-features-lacrosse.R(4 hunks)tests/testthat/test-plots-and-features-soccer.R(4 hunks)tests/testthat/test-plots-and-features-tennis.R(2 hunks)tests/testthat/test-plots-and-features-volleyball.R(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: pkgdown
- GitHub Check: lint
- GitHub Check: test-coverage
- GitHub Check: ubuntu-latest (oldrel-1)
- GitHub Check: ubuntu-latest (devel)
- GitHub Check: ubuntu-latest (release)
- GitHub Check: macOS-latest (release)
- GitHub Check: windows-latest (release)
🔇 Additional comments (12)
tests/testthat/test-plots-and-features-soccer.R (3)
6-7: LGTM! Improved test assertion using ggplot2 API.The change from manual class checking to
ggplot2::is_ggplot()is more robust and idiomatic.
16-17: LGTM! Consistent with modern ggplot2 testing practices.Good use of the official ggplot2 function for verifying plot objects.
31-32: LGTM! Proper ggplot object verification.The assertion correctly uses the ggplot2 API for object validation.
tests/testthat/test-plots-and-features-baseball.R (3)
6-7: LGTM! Clean modernization of ggplot verification.Excellent use of
ggplot2::is_ggplot()for robust plot object validation.
16-17: LGTM! Consistent test improvement.The change aligns with modern ggplot2 testing best practices.
36-37: LGTM! Proper ggplot object assertion.Good use of the official ggplot2 API for validating plot objects.
tests/testthat/test-plots-and-features-basketball.R (3)
12-14: LGTM! Excellent modernization of multiple plot assertions.Both plot objects are properly validated using the idiomatic ggplot2 function.
23-24: LGTM! Consistent with ggplot2 best practices.Proper use of the official API for plot object verification.
43-44: LGTM! Clean test assertion update.The change correctly uses
ggplot2::is_ggplot()for robust validation.tests/testthat/test-plots-and-features-football.R (3)
6-7: LGTM! Proper modernization of test assertion.Good use of
ggplot2::is_ggplot()for reliable plot object validation.
16-17: LGTM! Consistent test improvement.The change follows ggplot2 testing best practices effectively.
26-27: LGTM! Clean and idiomatic test assertion.Excellent use of the official ggplot2 function for object verification.
1 participant
See #43
Summary by CodeRabbit