Newest testthat (3.0) works ok on CLI, but VSCode gives green for all errors

[franzbischoff]

here is a lazy reprex:

./library(testthat)
./test_local()

/tests/testthat/test-some.R

content of test-some.R

test_that("some works", {
  expect_equal(1, 2)
})

test_local() works ok.
Using the VSCode UI, the error passes as a Green PASS

thanks

[RaphaelS1]

Thanks for this great extension!

I am also getting the 'green for all errors' bug too, except in my case the error started when I upgraded to R 4.1.0 from R 4.0.0. I had testtthat v3 in both versions (including when it worked).

[franzbischoff]

here is testthat 3.0 too, I also installed the last from github to check, but its the same.

version.string R version 4.1.0 (2021-05-18)
nickname       Camp Pontanezen  

[RaphaelS1]

So the bug is with R 4.1.0 not testthat 3.0?

[franzbischoff]

I have to check that...

[franzbischoff]

Tested with R 3.5.2, the same problem.
Installed testthat 2.3.2, and also the problem is there.

[meakbiyik]

@franzbischoff @RaphaelS1 Thanks for the heads up. I ran the unittest and encountered the error on CI, and now reproduced it on my local. It has something to do with how testthat reports the test results over the terminal, since I parse them with a nasty regex, which is indeed quite error-prone. Reading your comments, I am not quite sure whether the culprit is R 4.1.0 or testthat 3.0, but the former seems to be a better candidate since I also failed to reproduce with just the package updated. It is likely that when @franzbischoff tried to test the issue, the extension was using the wrong R, i.e. the one with version 4.1.0 -though of course this is just a guess.

I was quite aware of this particular soft spot, and need to fix it altogether soon but I am quite busy for a proper solution (see #7), so it will be just fixing the regex for now. Should be resolved soon.

[RaphaelS1]

Thanks so much for digging into this! Looking forward to the fix

[meakbiyik]

@franzbischoff @RaphaelS1 the issue should be resolved with the last version, do check it out and let me know if the issue persists.

I just hope that I have not broken support for the previous versions, though it is nasty to test that and I just cannot deal with the CI shenanigans right now.

[RaphaelS1]

Amazing thanks so much for the quick fix! Works perfectly.

Minor note that there is now a deprecation warning for test_file -> test_active_file

[franzbischoff]

I just wish to tanks the developer for tackling this issue so fast :)

[franzbischoff]

The problem seems to be back again :-(

Reports ok when should report failure...

[meakbiyik]

@franzbischoff Thanks for reporting it, I had pretty much delayed this issue with my last patch, and now need to fix properly with a streamer. I will handle it hopefully by the end of this week.

[meakbiyik]

This should be finally wrapped up, once and for all: the new reporter is much more robust than the old regex's and should work for at least until another major version of testthat. Thanks a lot @krlmlr for their contribution, much appreciated! Though, it also seems like I will not be maintaining this package actively anymore, so anyone should feel free to let me know if they want to be a developer.

@franzbischoff I am keeping this issue open until I hear from you - let's hope everything works out.

[franzbischoff]

@meakbiyik omw to test it!

[franzbischoff]

@meakbiyik Works well when using the "Native Testing" interface.
When using the Test Explorer interface, it works fine for testthat expectations, but not for checkmate::qexpect (the "Native Testing" gets the checkmate error).

Sample of the log of the error that is not detected by Test Explorer:

Check on data[[1]]$II is not TRUE

`actual` is a character vector ('Assertion on \'data[[1]]$II\' failed. Must be of length == 82502, but has length 82500.')
`expected` is a logical vector (TRUE)
Assertion on 'data[[1]]$II' failed. Must be of length == 82502, but has length 82500.

[franzbischoff]

@meakbiyik

Sample code:

test_that("Can read ECG file", {
  data <- read_and_prepare_ecgs(glue("{dataset_path}/a103l"))
  expect_equal(names(data[[1]]), c("time", "II", "V", "PLETH"))
  expect_equal(round(mean(data[[1]]$time) + sd(data[[1]]$time), 3), 260.261)
  expect_equal(round(mean(data[[1]]$II) + sd(data[[1]]$II), 3), 0.191)
  expect_equal(round(mean(data[[1]]$V) + sd(data[[1]]$V), 3), 0.982)
  expect_equal(round(mean(data[[1]]$PLETH) + sd(data[[1]]$PLETH), 3), 0.574)
  expect_equal(attributes(data[[1]]$II)$info$gain, 7247)
  expect_equal(attributes(data[[1]])$info$alarm, "Asystole")
  qexpect(data, "L1")
  qexpect(data[[1]], "L4")
  qexpect(data[[1]]$time, "N82500")
  qexpect(data[[1]]$II, "N82502") # here is the error
  qexpect(data[[1]]$V, "N82500")
  qexpect(data[[1]]$PLETH, "N82500")
})

[meakbiyik]

@franzbischoff thanks for the quick response! I played with checkmate a little but everything seems to work find on my side. Does the log mention nothing about an error? The code below for example:

test_that("Checkmate tests", {
  checkmate::qexpect(list("test"), "L1")
  checkmate::qexpect(c(2,3,4), "N2") # here is the error
})

produces failed result, with the following message:

Check on c(2, 3, 4) is not TRUE

`actual` is a character vector ('Assertion on \'c(2, 3, 4)\' failed. Must be of length == 2, but has length 3.')
`expected` is a logical vector (TRUE)
Assertion on 'c(2, 3, 4)' failed. Must be of length == 2, but has length 3.

I have never used checkmate with testthat before, so I am not sure of the expected error.

So in this order, if you have time, can you:

1. Check the extension version? It should be v0.5.1
2. Check the error message ("Show Log" decorator after test) and provide it here
3. And if possible create a reproducible example without dependencies?

Thanks a lot in advance :)

[franzbischoff]

@meakbiyik your example works... mine still not working :-/

1. Yes, I had it up to date :-)
2. below
3. let me see

Your code:

My code:

[franzbischoff]

something is off, now it seems not working at all again. only with the "Native UI".

Can you reach me on Telegram? user: franzbischoff

[meakbiyik]

Okay, the thing is the current code should not be able to provide you any sort of message if the test is passed, so this is completely unexpected. Does the old extension somehow linger in your workspace, interfering with the results? Reinstalling it might help, though no idea why. You can also see how the VSCode R Test Explorer extension output looks like - I think it was under debug console?

I am quite busy right now sadly, will try to take a look at this again these days. Let me know if you have a reproducible example and I will have a look. Thanks for your help!

[franzbischoff]

np, I'll leave the telegram link above. ping me when you have some time :)

[meakbiyik]

I think #16 will also solve your problem 🙂 Can you update devtools and check? I will close this issue if that's the case to avoid duplicates.

[franzbischoff]

Seems to continue the problem. Later I'll try again

[franzbischoff]

Here is a video

[meakbiyik]

Dear @franzbischoff, amazing video! Thanks a lot, and to be honest I hadn't even understood what you meant by "native" interface, apparently VSCode now provides a new API and the previous API extension just redirects to the native one if desired. So here's what I got: both native and non-native UI basically use the exact same extension, but only one of them works. Additionally, I clearly see a flip of results in non-native UI, something that should never happen. The extension runs correctly, detects error, reports it, but the report is immediately flipped.

Here's my guess: there's another copy of my extension somewhere in your PC, maybe because of some nasty thing VSCode has done during the update. Both extensions work, and the result depends simply on a race condition which the newer version wins (hence loses as only the last report is relevant). In native UI version, old extension simply doesn't work with the UI as it doesn't have the necessary dependencies, so only the updated extension works and provides the correct results.

Nevertheless, I will try to see if I can reproduce anything and will get back to you. Worst case I can simply migrate to native UI completely where everything works. Will let you know 👌

[franzbischoff]

1. the "Native" is the new interface that VSCode has now. Before the Test explorer had its interface, but VSCode now has its own, and the Test explorer has that option to use the old or the new.
2. Yes, the new (VSCode "native") works.
3. "I clearly see a flip": yes! I saw that too, in the old UI, right?

I don't mind using the new interface if this is the trend (I still prefer the old interface).

About the renv, do you mind adding in the debug output if the extension is using the correct environment or the global one? It's a way to debug further issues if we run into something that is related to renv.

Thanks a lot!

[meakbiyik]

@franzbischoff Thanks for all. I realized that at this point it makes no sense trying to force a deprecated API to work, so I am migrating to the Native UI where I know the extension works. This will take some time though as I am also cleaning the code at the same time, and the logic seems to be a bit different. Will keep you posted.

[franzbischoff]

Thanks @meakbiyik I'll use the Native UI from now on.
Again, thanks for this extension, let me know if I can help with something!