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

More tests, quietly #168

Merged
merged 16 commits into from
Dec 7, 2021
Merged

More tests, quietly #168

merged 16 commits into from
Dec 7, 2021

Conversation

maelle
Copy link
Member

@maelle maelle commented Dec 2, 2021

Fix #117
Fix #5
Fix #167

R/bump-version.R Show resolved Hide resolved
@maelle maelle requested a review from krlmlr December 2, 2021 10:01
@maelle maelle marked this pull request as draft December 3, 2021 14:09
@maelle
Copy link
Member Author

maelle commented Dec 3, 2021

@krlmlr I'll tackle #167 here hence the return to draft waiting for a decision in #174

maelle and others added 3 commits December 3, 2021 15:27
- `tag_version(force = FALSE)` succeeds when the tag exists and points to the head commit (#167).
@@ -24,7 +27,10 @@ collect_news <- function(messages) {
}
}

cli_alert_success("Found {.field {length(message_items)}} NEWS-worthy entries.")
if (fledge_chatty()) {
cli_alert_success("Found {.field {length(message_items)}} NEWS-worthy entries.")
Copy link
Member Author

Choose a reason for hiding this comment

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

btw I tried wrapping cli_alert() into another function but it was a mess because of the glue-like stuff in cli_alert() so it was simpler to use the conditional.

Copy link
Contributor

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Thanks, looks good. Would the tests benefit from local_demo_project() ?

@maelle
Copy link
Member Author

maelle commented Dec 6, 2021

I need to fix some tests actually 😅

@maelle
Copy link
Member Author

maelle commented Dec 6, 2021

such interesting failures 🙄

old[38:50] vs new[44:62]
  "  bump_version()"
  "Message <cliMessage>"
  "  > Scraping 4 commit messages."
  "  v Found 2 NEWS-worthy entries."
+ "  "
  "  -- Updating NEWS --"
+ "  "
  "  > Adding new entries to 'NEWS.md'."
+ "  "
  "  -- Update Version --"
+ "  "
  "  v Package version bumped to 0.0.0.9001."
  "  > Adding header to 'NEWS.md'."
  "  > Committing changes."
+ "  "
  "  -- Tagging Version --"
+ "  "
  "  > Creating tag v0.0.0.9001 with tag message derived from 'NEWS.md'."
  "  ! Call `fledge::finalize_version()`."
  "Output"

i.e. empty lines.

@maelle maelle requested a review from krlmlr December 6, 2021 12:11
@maelle
Copy link
Member Author

maelle commented Dec 6, 2021

@krlmlr the failure on Windows because Windows runs testthat 3.1.0 not testthat 3.1.1 😅

@maelle maelle marked this pull request as ready for review December 7, 2021 07:09
@maelle
Copy link
Member Author

maelle commented Dec 7, 2021

@krlmlr as this PR removes the empty lines it'd fix the problems on main.

For reference r-lib/testthat#1509

@krlmlr krlmlr merged commit e75bc1b into main Dec 7, 2021
@krlmlr krlmlr deleted the more-tests branch December 7, 2021 08:24
@krlmlr
Copy link
Contributor

krlmlr commented Dec 7, 2021

Thanks! Let's deal with the problems in demo.Rmd in a separate PR.

@maelle
Copy link
Member Author

maelle commented Dec 7, 2021

with the problems in demo.Rmd in a separate PR.

what problems?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

is this a bug Add a fledge.quiet option? Add tests
2 participants