Diagnostic messages not being returned in "dev" mode

[ncarchedi]

Diagnostic messages not being returned in "dev" mode

[ncarchedi]

This makes it challenging to test new content, since the instructor can't easily see why their submitted answers are failing.

[WilCrofter]

Most of the tests which use expectThat explicitly specify two labels which should appear in diagnostics. The one exception seems to be runTest.expr_identical. Is this the only offending case, or is the problem more general?

[WilCrofter]

Possible fix: labels for diagnostics added to runTest.expr_identical.

[ncarchedi]

I wasn't getting diagnostics for any of the tests. It's strange because I
know they were working before.

[ncarchedi]

Output from a question not using expr_identical() (in "dev" mode, Intro to R, module3):

| To add (or "concatenate") your name to the end of `my_char`, use the `c()`
| function like this: `c(my_char, "your_name_here")`. Place your name in double
| quotes where I've put "your_name_here". Try it now, storing the result in a
| new variable called `my_name`.

> 
> 3
[1] 3

| structure("Error in get(variable, e) : object 'my_name' not found\n", class =
| "try-error", condition = structure(list(
| message = "object 'my_name' not found", call = get(variable,
| e)), .Names = c("message", "call"), class = c("simpleError",
| "error", "condition")))

| 3

| 3

| length(c(3))

| That's not exactly what I'm looking for. Try again. Or, type info() for more
| options.

| Tack your name on to the end of the `my_char` vector using the `c()` function.
| Be sure to assign the result to a new variable called `my_name`. If your name
| was "swirl", you would type `my_name <- c(my_char, "swirl")`.

[WilCrofter]

On Tue, 28 Jan 2014 07:26:20 -0800
Nick Carchedi [email protected] wrote:

Output from a question not using expr_identical()

I'll try to track that down. Meanwhile, the diagnostics per se seem to be working:

| In it's simplest form, R can be used as an interactive calculator. Type 5 + 7
| and press Enter. Don't include the backticks (``) in your input. They are only
| there to denote an R command.

5+8
[1] 13

| 5 + 8 is not identical to 5 + 7. Differences: target, current do not match when
| deparsed

| Not exactly. Give it another go. Or, type info() for more options.

| Just type 5 + 7 (without the backticks) and press Enter.

[WilCrofter]

On Tue, 28 Jan 2014 07:26:20 -0800
Nick Carchedi [email protected] wrote:

Re:

Output from a question not using expr_identical() (in "dev" mode, Intro to R, module3)

Diagnostics are actually being printed. Note the |'s in the example, signifying the use of swirl_out. Admittedly, they are not very helpful in this case. The is_a diagnostic below is essentially a try error due to the non-existence of a variable my_name.

These tests were originally meant merely as proofs of the concept that testthat could be adapted for answer tests. If they are about to become a priority we can concentrate on bullet proofing them.

I can't promise much in the next week or so. I'll try to fix what I can.

DEBUGGING in Intro to R, module 3

| To add (or "concatenate") your name to the end of my_char, use the c()
| function like this: c(my_char, "your_name_here"). Place your name in double
| quotes where I've put "your_name_here". Try it now, storing the result in a new
| variable called my_name.

3
[1] 3

BREAKPOINT in runTest.is_a

Called from: eval(expr, envir, enclos)
Browse[1]> n
debug at /home/william/dev/r/swirlfancy/R/answerTests.R#203: results <- expectThat(try(get(variable, e), silent = TRUE), is_a(class),
label = label)
Browse[2]> n
debug at /home/william/dev/r/swirlfancy/R/answerTests.R#205: if (is(e, "dev") && !results$passed) swirl_out(results$message)

* INSPECTING results$message just passed to swirl_out:*

Browse[2]> results$message
[1] "structure("Error in get(variable, e) : object 'my_name' not found\n", class = "try-error", condition = structure(list( inherits from try-error not character"
[2] " message = "object 'my_name' not found", call = get(variable, inherits from try-error not character"
[3] " e)), .Names = c("message", "call"), class = c("simpleError", inherits from try-error not character"
[4] ""error", "condition"))) inherits from try-error not character"

[ncarchedi]

Are you running "dev" mode from the installed package or from your source (i.e. version controlled) directory? Does this make a difference?

[WilCrofter]

On Tue, 28 Jan 2014 07:26:20 -0800
Nick Carchedi [email protected] wrote:

Output from a question not using expr_identical() (in "dev" mode,

This particular problem was in runTest.is_a used in question 29, module 3, Intro to R. Just pushed a quick fix. Running in dev mode:

| To add (or "concatenate") your name to the end of my_char, use the c()
| function like this: c(my_char, "your_name_here"). Place your name in double
| quotes where I've put "your_name_here". Try it now, storing the result in a new
| variable called my_name.

3
[1] 3

| 3 inherits from numeric not character

| 3 does not use c

| 3 does not create a variable named my_name

| length(c(3)) not equal to 4L Mean relative difference: 0.75

| That's not exactly what I'm looking for. Try again. Or, type info() for more
| options.

| Tack your name on to the end of the my_char vector using the c() function. Be
| sure to assign the result to a new variable called my_name. If your name was
| "swirl", you would type my_name <- c(my_char, "swirl").

[WilCrofter]

On Tue, 28 Jan 2014 10:19:23 -0800
Nick Carchedi [email protected] wrote:

Are you running "dev" mode from the installed package or from your source (i.e. version controlled) directory? Does this make a difference?

I'm running the rebuilt, installed, and reloaded package in "dev" mode.

[WilCrofter]

On Tue, 28 Jan 2014 10:19:23 -0800
Nick Carchedi [email protected] wrote:

Are you running "dev" mode from the installed package or from your source (i.e. version controlled) directory? Does this make a difference?

Last reply was confused. My local master (source code in my version controlled directory) and swirldev/master are identical. (I have pushed the fix to swirldev.) My package is built and installed from my local master, hence functionally identical.

[WilCrofter]

On Tue, 28 Jan 2014 10:19:23 -0800
Nick Carchedi [email protected] wrote:

Are you running "dev" mode from the installed package or from your source (i.e. version controlled) directory? Does this make a difference?

Any luck?

[ncarchedi]

Just pulled changes from swirldev/swirl, built and reloaded:

> swirl("dev")

...

| To add (or "concatenate") your name to the end of `my_char`, use the `c()`
| function like this: `c(my_char, "your_name_here")`. Place your name in double
| quotes where I've put "your_name_here". Try it now, storing the result in a
| new variable called `my_name`.

> my_name <- 444

| 444

| my_name <- 444

| length(c(444))

| Keep trying! Or, type info() for more options.

| Tack your name on to the end of the `my_char` vector using the `c()` function.
| Be sure to assign the result to a new variable called `my_name`. If your name
| was "swirl", you would type `my_name <- c(my_char, "swirl")`.

> debug(swirl:::runTest.is_a)
debugging in: runTest.is_a(keyphrase, e)
debug at /Users/nicholasacarchedi/Dropbox/R_Working_Directory/swirl/R/answerTests.R#199: {
    temp <- strsplit(rightside(keyphrase), ",")[[1]]
    class <- str_trim(temp[1])
    variable <- str_trim(temp[2])
    if (!is.na(variable) && exists(variable, globalenv())) {
        val <- get(variable, globalenv())
    }
    else {
        val <- e$val
    }
    label <- val
    results <- expectThat(val, is_a(class), label = label)
    if (is(e, "dev") && !results$passed) 
        swirl_out(results$message)
    return(results$passed)
}

...

Browse[2]> n
debug at /Users/nicholasacarchedi/Dropbox/R_Working_Directory/swirl/R/answerTests.R#210: if (is(e, "dev") && !results$passed) swirl_out(results$message)
Browse[2]> is(e,"dev") && !results$passed
[1] TRUE
Browse[2]> str(results)
List of 5
 $ passed     : logi FALSE
 $ error      : logi FALSE
 $ failure_msg: chr "inherits from numeric not character"
 $ success_msg: chr "inherits from character"
 $ message    : chr "444 "
 - attr(*, "class")= chr "expectation"

What version of testthat are you using?

> packageVersion("testthat")
[1] ‘0.8’

It looks like results$failure_msg is what we're looking for. I bet we're running two different versions of testthat and that's why we're getting different results!

[WilCrofter]

On Tue, 28 Jan 2014 16:34:33 -0800
Nick Carchedi [email protected] wrote:

What version of testthat are you using?

> packageVersion("testthat")  
[1] ‘0.8’

packageVersion("testthat")
[1] ‘0.7.1’

Sorry. Will upgrade and fix ASAP.

[WilCrofter]

On Tue, 28 Jan 2014 16:34:33 -0800
Nick Carchedi [email protected] wrote:

What version of testthat are you using?

Actually, 0.7.1 is the current CRAN release, http://cran.us.r-project.org/web/packages/testthat/index.html. I would guess that's the right one to use for swirl at CRAN.

0.8 is the current version at github https://github.com/hadley/testthat/blob/master/DESCRIPTION.

[WilCrofter]

The easiest thing, involving the fewest changes to our code, would be to insert a fix in function expectThat, after line 365 of answerTests. Depending on the installed version of testthat, results will have either a message (0.7.1) or a failure_msg (0.8) member (see below.) I'd advocate creating a new structure which would include all the members in both versions, and have class attributes "expectation" and "swirl_expectation". That would be backward compatible with present code and forward compatible with future, provided testthat 0.8 doesn't change its expectation structure when it reaches CRAN.

What do you think? And, whatever we do, should the fix go in master immediately or in a branch?

Here are the two definitions of expectation:

testthat 0.7.1:
function (passed, message) { error <- is.na(passed) passed <- passed & !is.na(passed) structure(list(passed = passed, error = error, message = message), class = "expectation") }

testthat 0.8
function(passed, failure_msg, success_msg = "unknown") { error <- is.na(passed) passed <- passed & !error structure( list( passed = passed, error = error, failure_msg = failure_msg, success_msg = success_msg ), class = "expectation" ) }

[WilCrofter]

I've attempted to patch the problem in branch swirl_expectation, recently pushed to swirldev. It seems to work with testthat 0.7.1, but I have no convenient way to test it with 0.8. Awaiting comment.

[ncarchedi]

I'll pull and test with testthat 0.8. Stay tuned....

[ncarchedi]

Brilliant! I'm issuing a pull request with your changes. If you are comfortable merging, then go ahead and approve. Otherwise, feel free to comment on anything that you feel needs more review.

| To add (or "concatenate") your name to the end of `my_char`, use the `c()`
| function like this: `c(my_char, "your_name_here")`. Place your name in
| double quotes where I've put "your_name_here". Try it now, storing the
| result in a new variable called `my_name`.

> 2
[1] 2

| 2 inherits from numeric not character

| 2 does not use c

| 2 does not create a variable named my_name

| length(c(2)) not equal to 4L Mean relative difference: 0.75

| That's not the answer I was looking for, but try again. Or, type info() for
| more options.