Test-post_bug #7
Conversation
There was a problem hiding this comment.
Sorry for the late reply.
- Yes, it is better to just test the surface, i.e. only functions that are exposed to the user. This way the internals can change without changing tests.
- I don't understand this change. Why is it needed? When I ran it with graphics = FALSE and TRUE I got the same results on Rstudio and R terminal.
- The functions fails on purpose. Why do you wrap it on try? If an error is expected use
expect_error, or are you saying that expect_snapshot can't run when there is an error on the function?
R/post_help.R
Outdated
| } | ||
| invisible(answer) | ||
| invisible(answer == TRUE) |
There was a problem hiding this comment.
The need to change the return statement was that the invisible(answer) returns a string but the expect_equal() statement returns TRUE due to which when we we run devtools::test() it it always throws an error
There was a problem hiding this comment.
Change the expectation not the function. You can use expect_equal can be used to compare the output whatever it is.
tests/testthat/test-api.R
Outdated
| @@ -0,0 +1,48 @@ | |||
| # This is to check the create_bugzilla_key function | |||
There was a problem hiding this comment.
Why is this file being added again? Did you run pr_pull_upstream?
tests/testthat/test-post_bug.R
Outdated
| @@ -0,0 +1,7 @@ | |||
| cli::test_that_cli(configs = "plain", "post_bug() works", { | |||
| skip_if_not(interactive()) | |||
| url <- paste0(missing_host(), "rest/bug") | |||
There was a problem hiding this comment.
Great approach to build up the url, but the function is incomplete and still we need to think what will return to the user. That will depend on what returns the API. If possible it would be nice to return the ID of the bug to be able to look it up on the browser. (but don't test that on this PR)
Finally you added mockery to mock post_bug I don't see any mock function of post_bug being tested.
Ex:
menu(options, graphics = TRUE, title = title)Ex:
try(stop("Do it, later if you are still sure it is a bug then return please.", call. = FALSE))issue: #4