Skip to content

WIP: explore auto-labelling with values #2142

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions R/expect-equality.R
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,10 @@ expect_waldo_equal_ <- function(
)
if (length(comp) != 0) {
msg <- sprintf(
"%s (%s) is not %s to %s (%s).\n\n%s",
"%s is not %s to %s.\n\n%s",
act$lab,
"`actual`",
type,
exp$lab,
"`expected`",
paste0(comp, collapse = "\n\n")
)
return(fail(msg, info = info, trace_env = trace_env))
Expand Down
20 changes: 10 additions & 10 deletions R/expect-setequal.R
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ expect_setequal <- function(object, expected) {
if (length(exp_miss) || length(act_miss)) {
return(fail(paste0(
act$lab,
" (`actual`) and ",
" doesn't have the same values as ",
exp$lab,
" (`expected`) don't have the same values.\n",
".\n",
if (length(act_miss)) {
paste0("* Only in `actual`: ", values(act_miss), "\n")
},
Expand Down Expand Up @@ -138,11 +138,11 @@ expect_contains <- function(object, expected) {
if (any(exp_miss)) {
return(fail(paste0(
act$lab,
" (`actual`) doesn't fully contain all the values in ",
" doesn't fully contain all the values in ",
exp$lab,
" (`expected`).\n",
paste0("* Missing from `actual`: ", values(exp$val[exp_miss]), "\n"),
paste0("* Present in `actual`: ", values(act$val), "\n")
".\n",
paste0("* Missing from ", act$lab, ": ", values(exp$val[exp_miss]), "\n"),
paste0("* Present in ", act$lab, ": ", values(act$val), "\n")
)))
}

Expand All @@ -164,11 +164,11 @@ expect_in <- function(object, expected) {
if (any(act_miss)) {
return(fail(paste0(
act$lab,
" (`actual`) isn't fully contained within ",
" isn't fully contained within ",
exp$lab,
" (`expected`).\n",
paste0("* Missing from `expected`: ", values(act$val[act_miss]), "\n"),
paste0("* Present in `expected`: ", values(exp$val), "\n")
".\n",
paste0("* Missing from ", act$lab, ": ", values(act$val[act_miss]), "\n"),
paste0("* Present in ", act$lab, ": ", values(exp$val), "\n")
)))
}

Expand Down
40 changes: 36 additions & 4 deletions R/quasi-label.R
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,10 @@ quasi_label <- function(quo, label = NULL, arg = "quo") {
}

expr <- quo_get_expr(quo)
value <- eval_bare(expr, quo_get_env(quo))
label <- label %||% auto_label(expr, value)

labelled_value(
eval_bare(expr, quo_get_env(quo)),
label %||% expr_label(expr)
)
labelled_value(value, label)
}

labelled_value <- function(value, label) {
Expand All @@ -66,6 +65,39 @@ quasi_capture <- function(.quo, .label, .capture, ...) {
act
}

auto_label <- function(expr, value) {
if (is.call(expr) || is.name(expr)) {
label <- expr_label(expr)
if (can_inline(value)) {
paste0(label, " (", as_label(value), ")")
} else {
label
}
} else {
expr_label(expr)
Comment on lines +70 to +77
Copy link
Member

Choose a reason for hiding this comment

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

I'd avoid expr_label() as it's a remnant from lazyeval. Do you get good results if you use as_label() instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that and it introduced a bunch of small changes that were largely for the worse. I think this must have deviated from as_label at some time in the past.

}
}

can_inline <- function(x) {
Copy link
Member

Choose a reason for hiding this comment

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

rlang::is_syntactic_literal() might also be useful here? Would still need the nchar check.

if (is.null(x)) {
return(TRUE)
}
if (!is.atomic(x) || !is.vector(x)) {
return(FALSE)
}
if (length(x) != 1) {
return(FALSE)
}

if (is.character(x)) {
is.na(x) || (!grepl("\n", x) && nchar(x) < 100)
} else if (is.logical(x) || is.numeric(x)) {
TRUE
} else {
FALSE
}
}

expr_label <- function(x) {
if (is_syntactic_literal(x)) {
deparse1(x)
Expand Down
10 changes: 5 additions & 5 deletions tests/testthat/_snaps/expect-constant.md
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
# logical tests act as expected

FALSE (`actual`) is not equal to TRUE (`expected`).
FALSE is not equal to TRUE.
Copy link
Member

Choose a reason for hiding this comment

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

Should we quote the expressions?

Suggested change
FALSE is not equal to TRUE.
`FALSE` is not equal to `TRUE`.

I guess this becomes too heavy in the parenthetised/string-quoted cases

    x` (`1`) is not identical to `"a"`.


`actual`: FALSE
`expected`: TRUE

---

TRUE (`actual`) is not equal to FALSE (`expected`).
TRUE is not equal to FALSE.

`actual`: TRUE
`expected`: FALSE

# can compare non-vectors

quote(x) (`actual`) is not equal to TRUE (`expected`).
quote(x) is not equal to TRUE.

`actual` is a symbol
`expected` is a logical vector (TRUE)

# expect_null works

1L (`actual`) is not equal to FALSE (`expected`).
1L is not equal to FALSE.

`actual` is an integer vector (1)
`expected` is NULL

---

environment() (`actual`) is not equal to FALSE (`expected`).
environment() is not equal to FALSE.

`actual` is an environment
`expected` is NULL
Expand Down
8 changes: 4 additions & 4 deletions tests/testthat/_snaps/expect-equality.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
# provide useful feedback on failure

1 (`actual`) is not identical to "a" (`expected`).
`x` (1) is not identical to "a".

`actual` is a double vector (1)
`expected` is a character vector ('a')

---

1 (`actual`) is not equal to "a" (`expected`).
`x` (1) is not equal to "a".

`actual` is a double vector (1)
`expected` is a character vector ('a')

---

1 not identical to "a".
`x` (1) not identical to "a".
Types not compatible: double is not character

---

1 not equal to "a".
`x` (1) not equal to "a".
Types not compatible: double is not character

6 changes: 3 additions & 3 deletions tests/testthat/_snaps/expect-match.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

---

`one` does not match regexp "asdf".
`one` ("bcde") does not match regexp "asdf".
Text: "bcde"

---
Expand Down Expand Up @@ -47,11 +47,11 @@

# expect_no_match works

`x` matches string "e*".
`x` ("te*st") matches string "e*".
Text: "te*st"

---

`x` matches regexp "TEST".
`x` ("test") matches regexp "TEST".
Text: "test"

2 changes: 1 addition & 1 deletion tests/testthat/_snaps/expect-self-test.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
show_failure(expect_true(FALSE))
Output
Failed expectation:
FALSE (`actual`) is not equal to TRUE (`expected`).
FALSE is not equal to TRUE.

`actual`: FALSE
`expected`: TRUE
Expand Down
42 changes: 24 additions & 18 deletions tests/testthat/_snaps/expect-setequal.md
Original file line number Diff line number Diff line change
@@ -1,67 +1,73 @@
# useful message on failure

"actual" (`actual`) and "expected" (`expected`) don't have the same values.
"actual" doesn't have the same values as "expected".
* Only in `actual`: "actual"
* Only in `expected`: "expected"


---

1:2 (`actual`) and 2 (`expected`) don't have the same values.
`x` doesn't have the same values as `y` (2).
* Only in `actual`: 1


---

2 (`actual`) and 2:3 (`expected`) don't have the same values.
`x` (2) doesn't have the same values as `y`.
* Only in `expected`: 3


---

1:2 (`actual`) and 2:3 (`expected`) don't have the same values.
`x` doesn't have the same values as `y`.
* Only in `actual`: 1
* Only in `expected`: 3


---

c("a", "a") (`actual`) and c("b", "b", "b") (`expected`) don't have the same values.
`x` doesn't have the same values as `y`.
* Only in `actual`: "a"
* Only in `expected`: "b"


---

`x` doesn't have the same values as c("a", "b", "c", "d").
* Only in `expected`: "d"


# truncates long vectors

1:2 (`actual`) and 1:50 (`expected`) don't have the same values.
`x` doesn't have the same values as `y`.
* Only in `expected`: 3, 4, 5, 6, 7, 8, 9, 10, 11, ...


# expect_contains() gives useful message on failure

`x1` (`actual`) doesn't fully contain all the values in `x2` (`expected`).
* Missing from `actual`: "d"
* Present in `actual`: "a", "b", "c"
Comment on lines -42 to -44
Copy link
Member

Choose a reason for hiding this comment

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

That's actually a case where the parameter names are useful and add context (as opposed to e.g. object).

`x1` doesn't fully contain all the values in `x2`.
* Missing from `x1`: "d"
* Present in `x1`: "a", "b", "c"


---

`x1` (`actual`) doesn't fully contain all the values in `x3` (`expected`).
* Missing from `actual`: "d", "e"
* Present in `actual`: "a", "b", "c"
`x1` doesn't fully contain all the values in `x3`.
* Missing from `x1`: "d", "e"
* Present in `x1`: "a", "b", "c"


# expect_in() gives useful message on failure

`x1` (`actual`) isn't fully contained within `x2` (`expected`).
* Missing from `expected`: "a"
* Present in `expected`: "b", "c"
`x1` isn't fully contained within `x2`.
* Missing from `x1`: "a"
* Present in `x1`: "b", "c"


---

`x1` (`actual`) isn't fully contained within `x3` (`expected`).
* Missing from `expected`: "a", "b"
* Present in `expected`: "d", "e"
`x1` isn't fully contained within `x3`.
* Missing from `x1`: "a", "b"
* Present in `x1`: "d", "e"


8 changes: 4 additions & 4 deletions tests/testthat/_snaps/reporter-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@

== Failed tests ================================================================
-- Failure ('reporters/tests.R:12:3'): Failure:1 -------------------------------
FALSE (`actual`) is not equal to TRUE (`expected`).
FALSE is not equal to TRUE.

`actual`: FALSE
`expected`: TRUE
-- Failure ('reporters/tests.R:16:8'): Failure:2a ------------------------------
FALSE (`actual`) is not equal to TRUE (`expected`).
FALSE is not equal to TRUE.

`actual`: FALSE
`expected`: TRUE
Expand Down Expand Up @@ -71,12 +71,12 @@

== Failed tests ================================================================
-- Failure ('reporters/tests.R:12:3'): Failure:1 -------------------------------
FALSE (`actual`) is not equal to TRUE (`expected`).
FALSE is not equal to TRUE.

`actual`: FALSE
`expected`: TRUE
-- Failure ('reporters/tests.R:16:8'): Failure:2a ------------------------------
FALSE (`actual`) is not equal to TRUE (`expected`).
FALSE is not equal to TRUE.

`actual`: FALSE
`expected`: TRUE
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/_snaps/reporter-junit.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
</testsuite>
<testsuite name="Failures" timestamp="1999:12:31 23:59:59" hostname="nodename" tests="2" skipped="0" failures="2" errors="0" time="0">
<testcase time="0" classname="Failures" name="Failure_1">
<failure type="failure" message="FALSE (`actual`) is not equal to TRUE (`expected`). ('reporters/tests.R:12:3')">FALSE (`actual`) is not equal to TRUE (`expected`).
<failure type="failure" message="FALSE is not equal to TRUE. ('reporters/tests.R:12:3')">FALSE is not equal to TRUE.

`actual`: FALSE
`expected`: TRUE </failure>
</testcase>
<testcase time="0" classname="Failures" name="Failure_2a">
<failure type="failure" message="FALSE (`actual`) is not equal to TRUE (`expected`). ('reporters/tests.R:16:8')">FALSE (`actual`) is not equal to TRUE (`expected`).
<failure type="failure" message="FALSE is not equal to TRUE. ('reporters/tests.R:16:8')">FALSE is not equal to TRUE.

`actual`: FALSE
`expected`: TRUE
Expand Down
Loading
Loading