smoke tests

[jameslamb]

@bburns632 @jayqi looking for your feedback on this PR.

What it is

This is a proposal for finding issues with pkgnet. The code I introduce here runs pkgnet::CreatePackageReport() on every R package installed on your system.

Open Questions

1. Should this actually live in the pkgnet repo?
2. Should this code be modified to accept "a file with the names of some R packages", and then have separate documentation on how to generate such a file for "all installed R packages"?
3. If yes to 2, should we include these tests for some subset of packages in CI?
4. Anything that could be improved about the proposed code?

New Issues

By the way, I'm running this right now on my machine and will be adding some issues to the repo tonight. They'll all be linked by reference to this PR.

[codecov-io]

Codecov Report

Merging #252 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #252   +/-   ##
=======================================
  Coverage   92.44%   92.44%           
=======================================
  Files          12       12           
  Lines         927      927           
=======================================
  Hits          857      857           
  Misses         70       70

---

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 2f43f73...e0e0355. Read the comment docs.

[jameslamb]

After a finding a few errors in my test.sh code, I got a full run done on the packages on my system. Of the 951 packages installed on my machine (probably 945 of which are on CRAN), CreatePackageReport() with default arguments (except a specific report_path), the results were as follows:

• successes: 833
• failures: 118

Those failures are tracked in the following new issues:

• CreatePackageReport() fails with error "there is no package" #253
• CreatePackageReport() fails with certain packages that have R6 objects #254
• CreatePackageReport() fails on empty assignment in data.table() #255
• CreatePackageReport() breaks on bad logical condition #256
• CreatePackageReport() fails to generate report #257
• CreatePackageReport() fails with errors about loading some compiled libraries #258
• CreatePackageReport() fails to list objects in package #259
• CreatePackageReport() fails to determine dependencies between R6 objects #260

[jayqi]

This is great. I think (2) is a nice to have but isn't a big deal.

Running this in some kind of CI is a good idea. I wonder if we should add this as a job to build on the master branch. Or, if we want more flexibility (e.g., failures for this test don't fail our Travis build), we could maybe spin up another CI service and use that for running this test.

[bburns632]

I think (2) is a good solution. That would enable us to ensure pkgnet works/renders on of cran packages. Whether we run it on every build or on a scheduled basis is TBD.

[bburns632]

LGTM. We ready to merge this PR? Any last comments @jayqi ?

[jameslamb]

@bburns632 let me rebase to most recent master and try running it again, to make sure it still works. If that's so I'll change this from draft PR to ready for review.

[jayqi]

Approving, though I have some additional thoughts:

• If we have this in CI, I think it makes sense to run on a scheduled basis in addition to on push to master.
• If we're running in CI, maybe we should add (2).

We can always open an issue and do this later as a separate improvement. This current setup looks good.

Also, is it intended that the misc/smoke_tests branch is still listed in the GitHub Actions config? I actually think it may be a good idea to have a second branch listed. So if we want to manually run the smoke tests manually with additional packages or something (especially if we implement option 2 later), we can do that without having to make changes to master. May be worth documenting if we decide this is an intended feature.

[jayqi]

I'm thinking we should run this scheduled. Once every 2 weeks, or once a month, maybe? How long does it take to run?

[jameslamb]

ah! I think we should wait to see how long it takes before we decide. This check isn't actually passing yet...because this was created from a branch on my fork, you can only see the logs there: https://github.com/jameslamb/pkgnet/actions

If it runs on pushes to master, would it matter if it took 20 or even 40 minutes to run? I'm just making up that number...but since it wouldn't be blocking anything, in my opinion we might as well run it on even merge to master

[jameslamb]

ohhhh I see now that you said "IN ADDITION". Ok I agree with that, especially since we've slowed down developing here. Let me look into scheduled runs.

[jayqi]

I set it up last week for another project to run every Sunday, and it ran perfectly as expected this weekend. https://github.com/drivendataorg/deon/blob/b9fc9f6bf8318e90c68b0b89f3da6cba1a4c0592/.github/workflows/tests.yml#L7-L9

[jameslamb]

Ok @jayqi could you take another look? I finally came back and updated this.

Here's an example of the tests failing: https://github.com/jameslamb/pkgnet/runs/990800740?check_suite_focus=true. It's ok for them to fail, that tells us there are things that need to be improved (#252 (comment))

I took your suggestion to add a schedule to run it every Sunday. I like that!

Once you approve, I'll remove this branch from the list of branches in the workflow yaml

[jameslamb]

Also, is it intended that the misc/smoke_tests branch is still listed in the GitHub Actions config? I

I should have added a comment about that. I'm not proposing keeping that, I just did that as a way to test this PR.

I actually think it may be a good idea to have a second branch listed. So if we want to manually run the smoke tests manually with additional packages or something (especially if we implement option 2 later), we can do that without having to make changes to master.

My intention with the docs in this PR is to show how to run this manually yourself, in your local environment. I don't think it would be worth it to maintain another branch.

[codecov-commenter]

Codecov Report

Merging #252 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #252   +/-   ##
=======================================
  Coverage   92.60%   92.60%           
=======================================
  Files          12       12           
  Lines         946      946           
=======================================
  Hits          876      876           
  Misses         70       70           

---

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 330f379...bc9359a. Read the comment docs.

[jayqi]

Looks good to me.