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

Consider traceback of expectation failures #729

Closed
hadley opened this issue Mar 13, 2018 · 9 comments · Fixed by #921
Closed

Consider traceback of expectation failures #729

hadley opened this issue Mar 13, 2018 · 9 comments · Fixed by #921

Comments

@hadley
Copy link
Member

@hadley hadley commented Mar 13, 2018

And minimise impact of quasiquotation as much as possible.

@gaborcsardi
Copy link
Contributor

@gaborcsardi gaborcsardi commented Mar 13, 2018

Traceback would be great! Especially for things like expect_error_free(), etc.

@hadley
Copy link
Member Author

@hadley hadley commented Mar 29, 2018

Probably should use insights from lobstr::cst() to select more informative frames from stack.

@hadley
Copy link
Member Author

@hadley hadley commented Mar 28, 2019

And now insights from rlang

@lionel-
Copy link
Member

@lionel- lionel- commented Aug 2, 2019

Is this for compound expectations? We shouldn't get a backtrace in simple cases right? This doesn't seem very informative:

✔ |  OK F W S | Context
✖ |   2 2     | catch
────────────────────────────────────────────────────────────────────────────
test-catch.R:3: failure: bim bam boum
TRUE isn't false.
    █
 1. └─testthat::expect_false(TRUE) tests/testthat/test-catch.R:3:2

@lionel-
Copy link
Member

@lionel- lionel- commented Aug 2, 2019

We could print the trace only when it has more than one call:

expect_this_and_that <- function(x) {
  expect_true(x)
  expect_false(x)
}

test_that("bim bam boum", {
  expect_false(TRUE)
  expect_this_and_that(TRUE)
})
✔ |  OK F W S | Context
✖ |   3 3     | catch
────────────────────────────────────────────────────────────────────────────
test-catch.R:8: failure: bim bam boum
TRUE isn't false.

test-catch.R:9: failure: bim bam boum
Message: `x` isn't false.
Backtrace:
    █
 1. └─testthat:::expect_this_and_that(TRUE) tests/testthat/test-catch.R:9:2
 2.   └─testthat::expect_false(x) tests/testthat/test-catch.R:4:2

@gaborcsardi
Copy link
Contributor

@gaborcsardi gaborcsardi commented Aug 2, 2019

I would think this is most useful for expectations that take an expression, like expect_error(), expect_silent(), etc.

@jimhester
Copy link
Member

@jimhester jimhester commented Aug 2, 2019

Often the most important traceback is when there is an unexpected code error during a test.

If we could avoid having the testthat calls in the traceback in these cases it would clear up debugging greatly.

lionel- added a commit that referenced this issue Aug 7, 2019
lionel- added a commit that referenced this issue Aug 7, 2019
@lionel- lionel- mentioned this issue Aug 7, 2019
@lionel- lionel- closed this in #921 Aug 8, 2019
@lionel-
Copy link
Member

@lionel- lionel- commented Aug 8, 2019

When run from R CMD check, how about recording backtraces for all test failures, and creating a testthat-failures.Rout.fail file? This way we get backtraces for all unexpected results on the CRAN check page and on Travis.

@lionel-
Copy link
Member

@lionel- lionel- commented Aug 8, 2019

Scratch that, it wouldn't be very useful since the relevant call stack (from the code that produced the result) has already unwound at the time the expectation is signalled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants