-
Notifications
You must be signed in to change notification settings - Fork 5
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
Skip tests that depend on local db connection when it isn't present on testing machine #346
Skip tests that depend on local db connection when it isn't present on testing machine #346
Conversation
Or the API in the future...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 2 comments
.github/.gitignore
Outdated
@@ -0,0 +1 @@ | |||
*.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I typically avoid committing non-root .gitignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So do I! I didn't even notice this, it's usethis::use_github_action()
's doing. Do you think we should revert it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤷♂️ I typically revert it. Leaving it doesn’t break anything, might just be a bit confusinf why it’s there.
@@ -1,3 +1,5 @@ | |||
skip_if_not_localdb() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skips are typically done within test_that()
functions.
- Are you doing it at file level to avoid too much repetition?
- Does that indeed cancel any following tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed to avoid repetition, and it does work otherwise R CMD CHECK would fail on this branch 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll also make it easier to revert this if I pivot the tests to use the API instead of a local connection.
This PR introduces R CMD CHECK CI in github actions, skipping all tests that depend on the presence of a local database connection.
The Ci will also not build the vignettes or run the examples, as they will also require a local database connection. I added some arguments to the
rcmdcheck::rcmdcheck()
call in the github action to do this.AI Summary
Workflow improvements:
.github/workflows/R-CMD-check.yaml
: Added a new GitHub Actions workflow for R-CMD-check to ensure code quality and compatibility across different operating systems and R versions.Documentation updates:
README.Rmd
: Added a badge for the new R-CMD-check workflow to the documentation.man/etn-package.Rd
: Updated the funder information to "Research Foundation - Flanders".Testing enhancements:
tests/testthat/helper.R
: Added a helper functionskip_if_not_localdb
to skip tests if the ETN database is not a local database.tests/testthat/test-*.R
): Applied theskip_if_not_localdb
function to multiple test files to ensure tests are skipped when the ETN database is not available locally. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]Other changes:
.github/.gitignore
: Added*.html
to the.gitignore
file to ignore HTML files generated during the build process.tests/testthat/_snaps/download_acoustic_dataset.md
: Removed outdated snapshot test fordownload_acoustic_dataset()
.