-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fix failed pipeline #285
Fix failed pipeline #285
Conversation
✅ All contributors have signed the CLA |
I have read the CLA Document and I hereby sign the CLA |
Code Coverage Summary
Diff against main
Results for commit: d71c75d Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 18 suites 33s ⏱️ Results for commit d71c75d. ♻️ This comment has been updated with latest results. |
Scheduled workflow passed: |
Few points:
|
Or is |
Signed-off-by: Dony Unardi <[email protected]>
Unit Test Performance Difference
Results for commit 34c6428 ♻️ This comment has been updated with latest results. |
Yes, @pawelru did the research here
That was my first impression too, but we're only skipping this for workflows like noSuggest. Since we have other workflows with all dependencies installed, these tests are still being executed. If we want to upgrade these test cases to use regular base plots, I support it, but I'd prefer to handle it outside of this issue since it would require more attention. |
Fixes #284
Small note, I ended up using
testthat::skip_if_not_installed("ggplot2")
at the beginning of several unit test files (e.g.,test-DownloadReportModule.R
,test-LoadReporterModule.R
, etc.) because they depend onggplot2
at the start of the test.I don't think this update will affect our code coverage since code coverage is done using a different workflow (
Check
) which I believe will install all dependencies, includingggplot2
.