Add output unit test to pagedown

[felipecrp]

@cderv , @RLesur this PR add unit tests to pagedown, as discussed in PR #252.

It is still a work in progress. I just created the PR to share the progress of handling this issue.

We still need to generate the HTML from RMD. Also, the JEST is working with puppeteer, but the results are not as expected. I need to investigate.

Regards,

[cderv]

Thanks ! I passed your PR as draft to show that this is a working PR

[felipecrp]

Thanks. We can run the jest test using:

yarn install
./node_modules/.bin/jest

But the test is currently failing.

[felipecrp]

I really don't know (yet) why this testing is with error. I think it might be some puppeteer environment problem.

[felipecrp]

I solved the problem. Apparently, the puppeteer was not waiting for the page to completely load. Now, the test ended successfully.

[felipecrp]

@cderv, @RLesur,

It is working now!

The test:

• Generates the HTML file from Rmd;
• Parse the HTML and javascript using puppeteer;
• Assert the generated content using JEST;
• Delete the generated HTML file.

Now, we only need to insert JEST into the CI workflow. Can you help include the following commands in the CI workflow?

yarn install
./node_modules/.bin/jest

Regards,

[cderv]

Thanks a lot for the proof of concept. It is really interesting to know that this is working. Based on what you have done, I want to try to do that using R tools only (testthat + chromote) - I believe for R package it would be better to keep testing tools in the R ecosystem and not add new tools like Jest or puppeteer. This seems important to me on the upkeep side and if we want to generalize to other packages.

Hope you understand.

[felipecrp]

Hi @cderv,

I'm glad you liked the contribution. I understand that the proposed solution involves a change in the build process comprising the insertion of another language framework, which may be a problem for other contributors that do not know javascript.

However, I think that one strategy (full R solution) does not invalidate the other (R+javascript solution).

First, we have agreed that this is a complex issue to solve. Also, that regression tests are important to enable testing that future changes won't break already created (and tested) features. The proposed solution is an attempt to insert a regression test to pagedown to handle changes to the generated HTML file. It has its limitations and does not need to be the definitive solution. In fact, nothing prevents us to change to a full R solution in the future if we intend to. It may involve a test migration effort, but I think that enabling regression tests is more important.

Second, the proposed solution aims to test visual changes in the generated HTML file. Since paged.js already have regression tests, I think that the most important tests we need to write are related to hooks and hacks inserted by pagedown. These kinds of changes will mainly involve javascript instead of R. Hence, I think that using a Javascript test framework to validate javascript changes would be more intuitive (maybe, even for R developers). For instance, the long table test checks whether a javascript hook reinserts the THEAD when a table breaks.

Third, the proposed solution only affects the build process. Hence, pagedown users still don't need to set up a javascript environment. Also, developers that are just contributing with R code and that do not wish to set up a javascript environment may only run the R tests. The CI on GitHub will handle the javascript part and sound the alert if any change breaks the expected results.

Regards,

[felipecrp]

@cderv @RLesur ,

If the proposed solution is too much trouble for you, please feel free to reject/close this pull request.

Regards,

[cderv]

Hi @felipecrp,

The idea is really interesting and we want to consider it. We are not just working on pagedown right now and we need to discuss between maintainers on what we want to support. As I said, your idea can be implemented using the new tools you propose, or we can also use others.

Even if I agree with what you are saying

Third, the proposed solution only affects the build process. Hence, pagedown users still don't need to set up a javascript environment. Also, developers that are just contributing with R code and that do not wish to set up a javascript environment may only run the R tests. The CI on GitHub will handle the javascript part and sound the alert if any change breaks the expected results.

Introducing a new way of doing tests means that we need to support it, maintain it and then use it. It is just a decision we need to make in the team and that just takes time as we are currently working on other project right now.

Is there any rush on your side to merge such PR ?

[felipecrp]

Hi @cdev,

Thank you for the response. There is no rush at all. What motivated this PR was a discussion on #252. I will leave this PR open until you choose the test strategy.

Regards,