Fix/compliance #61
Merged
Fix/compliance #61
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| methods <- Map(function(g, c) test_has_methods(g, c, where), | ||
| key_methods, classes) | ||
| expect_more_than(length(classes), 0, info = paste0("No class in package ", pkg, " extends ", dbi_class)) |
There was a problem hiding this comment.
Please use expect_gt() and avoid using info (will be deprecated).
|
Could you please merge/rebase with master so that the tests can run? |
|
The build failed because the 'methods' package is not loaded. I think it should be added to the test call in the Makefile. I will try that. |
|
|
||
| classes <- Filter(function(class) { | ||
| extends(class, dbi_class) && getClass(class)@virtual == FALSE | ||
| }, getClasses(where)) |
There was a problem hiding this comment.
Do you need #' @importFrom methods getClass getClasses? Then it might work even without library(methods), which I'd prefer.
|
Indeed, I missed that. Added getClass and extends to importFrom. |
|
Thanks! |
krlmlr
pushed a commit
that referenced
this pull request
May 21, 2016
- Update documentation to reflect test condition (#70, @imanuelcostigan). - Support names for contexts (#67, @hoesler). - `simultaneous_connections` test always closes all connections on exit (@hoesler, #68). - More generic compliance check (@hoesler, #61). - Use container-based builds on Travis. - Install `RPostgres` and `RMySQL` from `rstats-db`.
krlmlr
pushed a commit
that referenced
this pull request
May 24, 2016
- Infrastructure
- Support names for contexts (@hoesler, #67).
- The `skip` argument to the test functions is now treated as a Perl regular expression to allow negative lookahead. Use `skip = "(?!test_regex).*"` to choose a single test to run (#33).
- Added encoding arguments to non-ASCII string constants (#60, @hoesler).
- Improve tests
- `simultaneous_connections` test always closes all connections on exit (@hoesler, #68).
- More generic compliance check (@hoesler, #61).
- Update documentation to reflect test condition (@imanuelcostigan, #70).
- `testthat` dependency
- Import all of `testthat` to avoid `R CMD check` warnings.
- Compatibility with dev version of `testthat` (#62).
- Improve Travis builds
- Use container-based builds on Travis.
- Install `RPostgres` and `RMySQL` from `rstats-db`.
- Install `DBI` and `testthat` from GitHub.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This compliance check is more generic.
In my DBI implementation hoesler/dbj, I am creating two different Result classes, depending on the type of the query.
The modified compliance check takes care of this. For all three DBI classes, it searches for classes in the package that extend them. Then, it checks for their presence and their methods.