[Suggestion] Add tests for the package check

[artemklevtsov]

I suggest to add some tests to improve stability.
There are test in the other packages base on the htmlwidgets. The main iea to check structure of the result object.

[JohnCoene]

Yes, I haven't found the time to just yet; hopefully sometime this year.

[etiennebacher]

I have some time to do something like that but I'm not sure of the things there are to test. We could check that the basic functions create objects that have both echarts4r or htmlwidget class, but I'm not sure that it would be useful.
I was thinking of testing the documentation examples like that:

library(testthat)

df <- data.frame(
  x = seq(50),
  y = rnorm(50, 10, 3),
  z = rnorm(50, 11, 2),
  w = rnorm(50, 9, 2)
)

test_that("e_line plot has the good classes", {
  plot <- df %>% 
    e_charts(x) %>% 
    e_line(z)
  
  expect_s3_class(plot, "echarts4r")
  expect_s3_class(plot, "htmlwidget")
})

What do you think? Would it be useful? I don't know what packages use htmlwidgets and how they test the functions.

[JohnCoene]

Yes, I meant to add tests but never found the time. This would be more than welcome!

Indeed, you would use testthat as you do, you cannot test the graphical fidelity only test on the object returned by e_charts: there's quite a lot to test to achieve decent coverage thought :)

[etiennebacher]

Okay I will add some tests. As you said, there will be many things to add, so I was thinking to make several PR so that people don't have to start from scratch if they want to contribute to this (since it is not very technical once you see the first tests). Is it okay for you or do you prefer a single big PR?

[JohnCoene]

Sure, you can it a few context/files at a time if you feel like it.

[etiennebacher]

In addition to the tests of class, I wonder if it would be possible to check if the JS (or json? I don't know) code produced by {echarts4r} matches the original code in echarts.js.

Suppose you want to test if your implementation of echarts.js in echarts4r is good. You could compare the "certified" code of echarts.js to the output produced by e_inspect() on the plot created by {echarts4r} (as in #199).

Is it possible? Relevant? Just throwing the idea here.

[swsoyee]

I have do some research about how to test when develop a {htmlwidgets} package, although I can not find any great example, also share some view here.

1. The benefit of testing the class of output maybe limited. There is a lot of cases is that the class of object is correct, but the graphs are not what the user expected. Many issue in this repository is reporting these problems.

2. You could compare the "certified" code of echarts.js to the output produced by e_inspect() on the plot created by {echarts4r} (as in String functions as arguments are supported. #199).

   Yes, I think it is the best test method for {echarts4r}. Use Jest test the option object in JavaScript side (may be too complicated) or {testthat} in R side (need to write a function to campare callback function option but would be not too hard).

3. Use snapshot testing in {testthat} 3.0 to make sure that every bug fix commit would not do any harmful side effects. I'm trying to use the 3rd edition of {testthat} but failed in my package development, so if some one know that how to use it would be helpful.

Also, there is an article that introduce other way to test {htmlwidgets}, but I haven't try them. I think {viztest} is doing the same job as snapshot testing but it can show the graphic result while snapshot testing is only compare the DOM structure. In the meanwhile {viztest} maybe costly.

[etiennebacher]

1. I think testing the class is important, as a failing test for this means that some functions are completely broken. But I agree, it is not enough.
2. I have never done JavaScript before so I'm not gonna use Jest but I'm okay to search for a test to do on R side. I don't understand what you mean by "compare callback function". Is it comparing the output of e_inspect() to original JSON code?
3. I wouldn't rely on {viztest} since the last commit was made more than 2 years ago, that there is no documentation and that it would add many dependencies. I tried to use snapshot testing but not very comfortable with it for now (I installed the development version of {testthat} (2.99.0.9000), and then added a line to DESCRIPTION as mentioned here).

[swsoyee]

I don't understand what you mean by "compare callback function". Is it comparing the output of e_inspect() to original JSON code?

When there is a callback function as options (for example, the formatter option below),

p <- mtcars %>%
  tibble::rownames_to_column("model") %>%
  e_charts(wt) %>%
  e_scatter(mpg, qsec, bind = model) %>%
  e_tooltip(
    formatter = htmlwidgets::JS(
      "function(params){
        return('<strong>' + params.name +
                '</strong><br />wt: ' + params.value[0] +
                '<br />mpg: ' + params.value[1])
                }
    "
    )
  )

p # plot

# extract the JSON
json <- p %>%
  e_inspect(json = TRUE,
            pretty = TRUE)

# print json
json

the result is a JavaScript function in string format with lots of \n and space, so a function should be implement to remove those \n and space, and it is able to compare the original setting option in JSON format of echarts.js.

"tooltip": {
    "trigger": "item",
    "formatter": "function(params){\n        return('<strong>' + params.name +\n                '<\/strong><br />wt: ' + params.value[0] +\n                '<br />mpg: ' + params.value[1])\n                }\n    "
  }

I tried to use snapshot testing but not very comfortable with it for now (I installed the development version of {testthat} (2.99.0.9000), and then added a line to DESCRIPTION as mentioned here).

I have try it before, but I can not enable the 3rd edition and use the snapshot testing, so I gave up and just waiting for the official release of 3.0 😢

[JohnCoene]

I think a key thing to test is the data structure produced by echarts4r for each "geom," expect_s3_class only test that the object returned is indeed of the class but everything else could be wrong and the test would still pass.

Tests should be rather strict, the data looks like the place to start to me.

p <- head(cars, 2) %>% e_charts(speed) %>% e_scatter(dist)

testthat::expect_equal(p$x$opts$series[[1]]$data, list(list(value = c(4, 2)), list(value = c(4, 10))))

[etiennebacher]

Agreed, it could also be useful to add similar test for p$x$opts$series[[1]]$type or p$x$opts$series[[1]]$coordinateSystem, what do you think?