pagedown
pagedown copied to clipboard
Upgrade paged.js to version 0.2.0
I upgraded the paged.js from version 0.1.43 to 0.2.0.
I noticed that we use the paged.polyfill.js instead of the paged.js file. Hence, I renamed the file to make future upgrades straightforward.
Regards,
Hi @RLesur, I did just minor tests with my documents. I agree that more tests are required.
I saw some unit tests on the project, but they apparently test some render features but do not assert whether pagedown created the output correctly. It would be nice to have automated regression tests, but I understand that this might be difficult because we need to compare the generated files visually.
We need to think about how to create some tests to enable regression before major changes. Do you have any suggestions?
Regards,
@felipecrp Having the tools to perform visual tests against regressions would be great. The .test parameter introduced in https://github.com/rstudio/pagedown/commit/031b1cb8b8bbcb96353ba2a17c589a7eaddfbf40 and https://github.com/rstudio/pagedown/commit/d6d649d20b66dfd3a173247af644ecab916984c9 was the very first step in this direction. We could compare the PDF files or pages screenshots. I did some attempts 2 years ago to compare pages screenshots and had serious issues while automating the process. Maybe comparing the PDF files would be an easier move.
@RLesur some tests can also explore the generated DOM. For instance, a javascript code could check whether a header is repeated on every page or if a footnote is presented at the bottom of the page.
I just saw that the paged.js use this strategy (DOM check) for unit testing.
https://gitlab.pagedmedia.org/tools/pagedjs/tree/master/specs
They basically have an HTML and a javascript file that check whether the result is ok. I think we could do the same, but with RMarkdown.
@felipecrp You're right, DOM checking could be another strategy. However, I wonder whether this would be useful in the context of pagedown. For instance, in the context of the current PR, DOM checks would warn that the DOM have changed (because the Paged.js' page box model changes), this wouldn't be a great help. But feel free to send a proposal!
@RLesur I think we only need to unit test the 'hacks' included on pagedown, e.g., the hooks or custom behaviors.
Automated more tests would be awesome ! But testing R Markdown ecosystem is really not easy. Especially pagedown which acts at the client level with lots of JS in the browser. FWIW paged.js is using JEST JS framework (https://jestjs.io/) for its testing, but it is not something easy for us to use directly in our ecosystem. Anyway, this auto-testing topic of R Markdown output is something broader that I would like to tackle in the future so happy to take any idea and aspiration. Putting here some idea that I believe we could need to use something like to stay in the R ecosystem
- Use testthat as a testing framework in R
- Use chromote or crrri for load the HTML rendered using chromium
- Dump the DOM in a new HTML file & use xml2 to parse and check the DOM expectations
We could also switch to a JS framework for that but it would need to run R in the first step and we would need to develop more JS skills. From paged.js test workflow:
- Use Jest or another framework to run tests in JS
- Run R from JS to render Rmd to HTML
- Use pupetteer to render the page in a browser for JS to work
- Use JS code within the jest framework to check the content of the rendered HTML
I don't know if not using R as advantage here for testing R package.
Last solution we already think about is a printing to PDF or image and doing a visual comparison but this has other challenges and if we detect an issue, it would need manual visual inspection for find the difference, then the part in the DOM, then the code that fail.
Anyway, interesting topic as you see! 😄
but after all , I think this would still not prevent us to do some manual visual checks of our template and pagedreport template. Not everything change can be auto test. What we could improve right now is make a checklist, and some helper functions so that we can easily run manual test without much trouble and know what to look into. I believe this would be the first step before
I was trying to set up the test environment yesterday, using JEST but got some issues with my WSL installation to use puppeteer.
Since we need to test the DOM, I believe that it will be easier to use a JS test framework, such as JEST. Of course, we need to start from an Rmd file, then we need to use a workflow like the following:
- Parse Rmd and generate the HTML file
- Run the HTML file and assert the generated DOM
Maybe, the easier way to accomplish this is to use testthat just to generate the temporary HTML files and JEST to consume the temporary HTML, assert, and delete. We just need to assert that testthat would run first. Also, we could try to use some JEST before assignments to call R, but I think this could be more complicated.
PS: I think this discussion deserves another PR/issue because it is a much complex issue than the upgrade itself.
Regards,
Since we need to test the DOM, I believe that it will be easier to use a JS test framework, such as JEST.
FWIW testing the DOM content using xml2 and XPATH request is what we do in other tools to test that the created DOM is the one expected. Also chromote or crrri would be enough to do like puppeteer from R directly.
PS: I think this discussion deserves another PR/issue because it is a much complex issue than the upgrade itself.
Agreed I share my thoughts here because I was under the impression you would have already tried something - and I was right 😉
Please feel free to open a new issue to share your idea, your progress and thoughts on all this. I don't know what the best solution would be. I just know this is not something pagedown specific and it is more broader problem. But trying something with pagedown is definitely worth it !
Hi @cderv,
I just created the #253 to share my progress (still work in progress).
Regards,