-
-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible to optionally skip Pandoc testcase? #99
Comments
Is it your local environment? I have released v0.6.2 yesterday on CRAN, and it seems at least https://cran.r-project.org/web/checks/check_results_ftExtra.html |
@atusy Thank you for responding. That was on GitHub CI, because I did not require Pandoc dependency. However we do not want to require it, for the reasons I mentioned: at the very least, it is just unavailable for some platforms where modern GHC and friends are broken. Everything else works fine, only Pandoc testcase fails. It won’t show up on CRAN, since they, I guess, pre-install Pandoc for their checks. This is not a bug, of course, but it makes tests fail unnecessarily, which otherwise would pass. |
For example, we skip rebuilding vignettes and manuals in order to avoid dependency on other heavy and unnecessary for R itself stuff like Tex. (Tex is not broken, but it is a huge thing to build.) |
Why do you perform ftExtra's tests on somewhere else...? |
We have ftExtra in Macports :) |
Ah hah! Now I see. testthat package has skip features. https://testthat.r-lib.org/articles/skipping.html. I want to avoid unintentional skip of tests. |
@atusy Basically anything which would allow tests to pass normally without Pandoc requirement would solve the problem. We can pass an argument or set an environment variable. I do not suggest to change default behavior, just add an option to avoid Pandoc if a user or downstream distribution prefers. |
I guess a quick and robust solution is adding
I can consider skip feature, but the release will be four weeks later because I released the latest on yesterday.
|
There are two options, but I do not like both...
|
@atusy The issue is not about hacking CI in fact: I am adding a fix now for R portgroup which would ensure, I think, that tests are not run unless enabled. And then disabling tests gonna work again, and CI will pass. However, given that all tests but one pass without Pacdoc, I thought we could find a reasonable way to enable tests, but without requiring Pandoc. And if they are enabled, then a failure becomes a problem for CI. It may not be worth fixing this issue, if that requires a substantial rewrite of the code or having trade-offs in terms of code quality. In the worst case, I can make a local patch to get rid of Pandoc stuff, or just keep tests disabled. P. S. To make CI work with Pandoc, portfile just has to declare dependency on Pandoc, which will result in CI installing Pandoc prior to building and testing |
@atusy As a quick fix to make updating |
Tests fail to pass if Pandoc is not installed:
However, Pandoc is broken on some systems, and a non-trivial dependency to build. Is it possible to conditionally skip it or may it non-obligatory?
The text was updated successfully, but these errors were encountered: