-
Notifications
You must be signed in to change notification settings - Fork 80
Add workflows for tests and R CMD check
#262
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
Conversation
@philchalmers it might be worth considering splitting the test workflow into multiple smaller workflows, running smaller sunsets of the tests. E.g., one workflow running test files 00-03, 04-06, ... and so on. That way the tests would probably complete a bit faster, and you could choose to only run relevant tests on pushes/pull-requests. |
Great, thanks! I'll give this new workflow a spin for a while to see how it works. Thanks for the code quality contributions. |
Something in this pull seems to have triggered a check issue on my local version. Could you check it out? It looks to be related to a Debian specification somewhere.
|
Ignore that; this seems related to my Ubuntu desktop rather than this pull. On other OS instances (Ubuntu server included) this looks fine. |
I see,
I see that the same *note* is present on the workflow as well. As you
suggest, it's caused by the platform. I think the compilation flags are
inherited from the compilation flags used to build R itself -- since I
wasn't able to deactivate the compilation flags by changing the Makevars
file, or the environment variables. I updated the container image to use a
different R-image (rocker/r-base). Now there should no longer be a note on
non-portable compilation flags (on Github atleast).
I completely forgot to mention this in my pull-request:
Currently the image used in the workflow is hosted from a repo on my Github
account (https://github.com/Kss2k/container-mirt). The reason for making a
container in the first place is to avoid having to download (and compile)
dependencies every time you run the checks.
For the time being I've sent you an invite to be a collaborator on the
repo. Once you accept, I intend to make you an administrator. If you wish I
could downgrade my own role. We could also add branch protection rules,
such that you need to approve any future updates/commits. If this is not
satisfactory, it might be a good idea for you to copy/fork it, and host it
from your own account (+ replacing the container in your workflow file).
Best,
Kjell
man. 11. nov. 2024 kl. 22:51 skrev Phil Chalmers ***@***.***>:
… Ignore that; this seems related to my Ubuntu desktop rather than this
pull. On other OS instances (Ubuntu server included) this looks fine.
—
Reply to this email directly, view it on GitHub
<#262 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3BNAT4VX5CMQKUUYJZCVVT2AERHJAVCNFSM6AAAAABRSR6DDSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRZGEYTIMZSGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Hi Kjell, I've decided to go with the latter approach of hosting the docker image on my account by copying your current codebase, though I've not done this on Github before and am likely to hit some bumps along the way..... Nevertheless, I greatly appreciate all the information you've provided. All the best, Phil |
Good idea!
PS: If you wish to host the container publicly, it is not enough to make
the repo public. You must also make the package (I.e., the container)
public. I've forgotten this a few times my self...
Best,
Kjell
…On Tue, 12 Nov 2024, 18:38 Phil Chalmers, ***@***.***> wrote:
Hi Kjell,
I've decided to go with the latter approach of hosting the docker image on
my account by copying your current codebase, though I've not done this on
Github before and am likely to hit some bumps along the way.....
Nevertheless, I greatly appreciate all the information you've provided.
All the best,
Phil
—
Reply to this email directly, view it on GitHub
<#262 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3BNAT4XBSD4AZLUC7ZILST2AI4LHAVCNFSM6AAAAABRSR6DDSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINZRGE3TKNBYGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Purpose
In reference to the email I sent previously, I am submitting this pull request for the commits I've made to the repository. This pull request includes the addition of workflows that enable the
R CMD check
and package tests to run automatically on pull requests.The main purpose of this pull request is to demonstrate how these workflows can be activated on pull requests. Please feel free to review the changes. If you have any concerns or suggestions, don't hesitate to request modifications or reject the changes as you see fit.
Changes
I added two workflows for
R CMD check
and running tests usingtestthat
(viadevtools::test
). Both workflows are set to run on pull requests (if approved by a maintainer).R CMD check
is automatically triggered on push to main. Since the tests take quite a long time to run (approximately 45-50 mins), the tests workflow needs to be manually triggered by a maintainer onGitHub
. This can be changed if you wish.When creating the workflows I made some changes in regards to the
R CMD check
and the tests. For theR CMD check
I added a new line to theDESCRIPTION
file, specifying theEncoding
(UTF-8
), 'required' byroxygen2
, when writing the documentation.Regarding the tests a lot of changes were made. Firstly I renamed
tests/tests/
totests/testthat/
, to make the tests runable bydevtools::test()
.testthat
automatically added some lines to theDESCRIPTION
file. I also updated some of the test-files, in cases where there were deprecation warnings (e.g,. "tol
is deprecated, usetolerance
instead", "context()
is deprecated, and can be removed"). Additionally I added some global settings toR
among them is warnings for partial matches. I.e., when you retrieve an object from alist
using the$
operator, you may sometimes retrive it by a partial match (e.g.,mylist$groupName
may retrieve the partial matchmylist$groupNames
). I located some of these partial matches in the package, and corrected them accordingly.Currently, the test workflow results in 4 errors, which I think stem from the package/tests themselves, and not from any of my commits.