Skip to content
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

try render and check for pdflatex #108

Merged
merged 15 commits into from
Jul 8, 2022
Merged

try render and check for pdflatex #108

merged 15 commits into from
Jul 8, 2022

Conversation

Polkas
Copy link
Contributor

@Polkas Polkas commented Jul 6, 2022

closes #89

  • Add a validation if the pdflatex is installed, if not raise an error.
  • try call around the render call.
  • rm sprintf where not needed.

tested with podman run --rm -p 8787:8787 -e PASSWORD=yourpasswordhere rocker/rstudio as it not contains pdflatex by default.

@Polkas Polkas added the core label Jul 6, 2022
@Polkas Polkas marked this pull request as ready for review July 6, 2022 16:17
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2022

Unit Tests Summary

    1 files    17 suites   10s ⏱️
186 tests 186 ✔️ 0 💤 0
268 runs  268 ✔️ 0 💤 0

Results for commit 203f9c1.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2022

Code Coverage Summary

Filename              Stmts    Miss  Cover    Missing
------------------  -------  ------  -------  -------------------------------------------------------------------------
R/AddCardModule.R       100       3  97.00%   75-77
R/Archiver.R             25       0  100.00%
R/ContentBlock.R         16       0  100.00%
R/DownloadModule.R      161      40  75.16%   77-79, 100, 150-155, 164-168, 171-175, 183-187, 190-194, 201-205, 208-212
R/FileBlock.R            13       0  100.00%
R/NewpageBlock.R          2       0  100.00%
R/PictureBlock.R         30       1  96.67%   15
R/Previewer.R           217      11  94.93%   162, 164-167, 172-178
R/Renderer.R             54       4  92.59%   142, 144-145, 166
R/ReportCard.R           76       2  97.37%   211, 232
R/Reporter.R             96       1  98.96%   255
R/ResetModule.R          45       0  100.00%
R/SimpleReporter.R       18       0  100.00%
R/TableBlock.R            8       0  100.00%
R/TealReportCard.R       20       0  100.00%
R/TextBlock.R            13       0  100.00%
R/yaml_utils.R           74       2  97.30%   65, 263
TOTAL                   968      64  93.39%

Results for commit: cdb97cd

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@mhallal1 mhallal1 self-assigned this Jul 7, 2022
R/DownloadModule.R Outdated Show resolved Hide resolved
@mhallal1 mhallal1 removed their assignment Jul 7, 2022
@mhallal1
Copy link
Contributor

mhallal1 commented Jul 7, 2022

Looks fine to me after the fix of system2 and works as expected.
I did not test it with an environment without pdflatex as my docker is not very happy therefore I will leave it to be approved by someone who could test this. @junlueZH ?
@npaszty I believe you had this issue also so it would be great if you could test this PR locally?

Copy link
Contributor

@mhallal1 mhallal1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed with @Polkas shared screen how he tested this on a the docker image above and it worked as expected.

@mhallal1 mhallal1 self-assigned this Jul 7, 2022
@Polkas Polkas merged commit aa79608 into main Jul 8, 2022
@Polkas Polkas deleted the 89_tryrender@main branch July 8, 2022 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Capture renderer error and notify user
2 participants