`cli::cli_alert_success()` fails `testthat::expect_message()`, while `cli::cli_inform()` doesn't

[peterdesmet]

I had to revert cli::cli_alert_success() to cli::cli_inform() in shift_time() in https://github.com/inbo/camtrapdp/pull/108/commits/093e85fc819447de83c5260ddcd51ed8fe87b909.

The reason is that cli::cli_alert_success() failed the testthat::expect_message() tests. Both the test for the class (is class not supported for cli_alert_success() ?) and the test with regexp (is the wrap = TRUE not working as expected?).

This is not an important bug, users just won't get a nice ✔ in front of the message. It is an odd bug though, that I find hard to reproduce.

For example, a small reprex example does pass the tests:

library(cli)
library(testthat)

# cli::cli_inform()
my_inform <- function() {
  cli::cli_inform(
    "cli inform message",
    class = "my_inform"
  )
}
my_inform()
#> cli inform message
testthat::expect_message(
  my_inform(),
  class = "my_inform"
)
testthat::expect_message(
  my_inform(),
  regexp = "cli inform message",
  fixed = TRUE
)

# cli::cli_alert_success()
my_alert <- function() {
  cli::cli_alert_success(
    "cli alert
     message",
    wrap = TRUE,
    class = "my_alert"
  )
}
my_alert()
#> ✔ cli alert message
testthat::expect_message(
  my_alert(),
  class = "my_alert"
)
testthat::expect_message(
  my_alert(),
  regexp = "cli alert message", # Fails when using "v cli alert message"
  fixed = TRUE
)

^{Created on 2024-09-05 with reprex v2.1.0}

So I don't understand what is causing the tests to fail in shift_time(). Note that an unassigned call to shift_time() will return two messages (the message + the default print), but that is not causing the issue.

library(camtrapdp)
x <- example_dataset()
deployment_id <- "00a2c20d"
duration <- lubridate::duration(-4, units = "hours")
shift_time(x, deployment_id, duration)
#> Date-times in selected deployments, media and observations were shifted by
#> -14400s (~-4 hours). E.g. 2020-05-30 02:57:37 is now 2020-05-29 22:57:37.
#> A Camera Trap Data Package with 3 tables:
#> • deployments: 4 rows
#> • media: 423 rows
#> • observations: 549 rows
#> 
#> And 1 additional resource:
#> • individuals
#> Use `unclass()` to print the Data Package as a list.

^{Created on 2024-09-05 with reprex v2.1.0}

[PietrH]

For future reference we should note the commit where the test for shift_time() fails, or maybe include logs of a github actions run?

[peterdesmet]

Sure, the two commits before my correction to cli::cli_inform() fail:

https://github.com/inbo/camtrapdp/commit/44cebe6ed4ba855528fdca45c048989ba8dd0bbd https://github.com/inbo/camtrapdp/commit/f2338dceb1f5d11750e75f5baf52ff076401eb9e

@sannegovaert 's original use of cli::cli_alert_success() did not fail:

https://github.com/inbo/camtrapdp/commit/18f34e01a97e37a124d507f02bc2f726aa67d4b4

[PietrH]

If I capture the message class with cli_alert_success(), I don't get the expected message class of camtrapdp_message_shift_time:

tryCatch(shift_time(x, deployment_id, duration), message = function(m){print(class(m))})
#> [1] "cliMessage"    "simpleMessage" "message"       "condition"    

While with cli_inform(), I do:

tryCatch(shift_time(x, deployment_id, duration), message = function(m){print(class(m))})
#> [1] "camtrapdp_message_shift_time" "rlang_message"               
#> [3] "message"                      "condition"   

I also noticed that cli_alert_success() results in a simpleMessage and cli_inform() in a rlang_message, I was under the impression that all cli messaging was using rlang under the hood; https://rlang.r-lib.org/reference/abort.html

In cli_inform() the class argument is actually passed to rlang via ... tripledots/ellipsis. While in cli_alert_success(), class is a defined argument

Via a helper, this class (of the message) is set here: https://github.com/r-lib/cli/blob/d33aeeee137bba67c423bc3e03949e45c48b8157/R/cli.R#L987-L991

I'm getting the feeling that while class from rlang and class from cli sound the same, they do different things. I wonder what is meant exactly by alert, it's not clear how they differ from the other functions in cli.

Thus ends my investigation, I'm not convinced the added value of cli_alert_success() is worth the hassle in this case. I'm going to leave it here, feel free to reassign me if you want to revisit this.

[peterdesmet]

Thanks for investigating. Closing this, ✔ can be replicated with "v" = "My message...", see #116.