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

mrc-257: update tests to work with changes to R 3.6.0 and testthat #170

Merged
merged 9 commits into from May 9, 2019

Conversation

@richfitz
Copy link
Member

@richfitz richfitz commented May 9, 2019

A long sad story:

  • R 3.6.0 now checks that paths are not hard coded in a package installation to support staged installs blog post which started throwing errors in the package code because of how the IR is loaded from disk. This has been changed to load-on-demand for now (this is the only substantive change to the package)
  • testing this requires 3.6.0 which is done on travis but not debuggable so I am doing that with docker - so we add support for docker there
  • we'd have picked this up faster if we tested against different R versions automatically so I have expanded the travis testing
  • the actual test failures are very difficult to find through a change in testthat's behaviour so 27a7601 patches a lot of tests to keep it quiet
richfitz added 7 commits May 8, 2019
Change introduced in
r-lib/testthat#816

requires update to ~200 tests as warnings make identification of
test failures almost impossible.
This was used to debug this mrc-257 by making it easier to use
the newest R
@richfitz richfitz requested a review from weshinsley May 9, 2019
@codecov
Copy link

@codecov codecov bot commented May 9, 2019

Codecov Report

Merging #170 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #170   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          38     38           
  Lines        4510   4504    -6     
=====================================
- Hits         4510   4504    -6
Impacted Files Coverage Δ
R/utils.R 100% <ø> (ø) ⬆️
R/interface.R 100% <100%> (ø) ⬆️
R/generate_c_class.R 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2043f38...70b24f7. Read the comment docs.

richfitz added 2 commits May 9, 2019
Copy link
Contributor

@weshinsley weshinsley left a comment

All looks good - that's indeed a lot of updated tests...

@weshinsley weshinsley merged commit ffb0ec0 into master May 9, 2019
6 checks passed
6 checks passed
codecov/patch 100% of diff hit (target 100%)
Details
codecov/project 100% (+0%) compared to 2043f38
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.