Skip to content

Split dependency building and pkg checking on travis #368

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

Merged
merged 6 commits into from
Oct 24, 2018
Merged

Split dependency building and pkg checking on travis #368

merged 6 commits into from
Oct 24, 2018

Conversation

sgibb
Copy link
Collaborator

@sgibb sgibb commented Oct 19, 2018

Currently our CI fails on travis because the hard runtime limit for jobs
is 50 minutes and installing all dependencies of MSnbase takes a lot of
more time.

image

This commit introduces multiple changes:

  • Use 2 cores for package building/installation.
  • Split building of dependencies and package checking into different
    jobs which allows us to use 2x50 minutes. The build job builds all dependencies and create a cache which is used by the check job.
  • Automatically deploy of pkgdown based documentation to github pages the master branch.

@lgatto if you want to use the automatic deployment you have to create a Personal Access Token with the public_repo scope/permission and register it in the travis' settings with the name GITHUB_PAT

We could create even more jobs to reduce the check time a little bit (currently after cache building it takes ~ 35 min to run travis but using three check jobs ("examples", "vignettes", "test") we could go down to ~ 25 min:

2 jobs (build + check, current PR)

travis1

5 jobs (build + 3 check + codecov) (the codecov job failed because I accidentally used rscript instead of Rscript)

tavis2

@lgatto If you wish I could easily extend this PR to the version with 5 jobs.

BTW:

  • I add skip_on_travis() to tests/testthat/test_MSmap.R on my forked repository because downloading files from AnnotationHub always failed (maybe we should consider it to add it here, too).
  • MSnbase needs more than 250 packages to build. Maybe we could remove a few, e.g. shiny is just used for one function (could selectFeatureData move to pRolocGUI?), same for XML which is just used by .isCentroidedFromFile and mzID both could be replaced by mzR/xml2 (the latter is downloaded anyway by other dependencies), affy/affyio just for MAplot, gplots just for heatmap.2 in the vignette, ... it is somehow connected to speed up package load time #201 but would just improve the build time. Should we open an issue and try to remove some build dependencies or just keep status quo?

Currently our CI fails on travis because the hard runtime limit for jobs
is 50 minutes and installing all dependencies of MSnbase takes a lot of
more time.

This commit introduces multiple changes:
- Use 2 cores for package building/installation.
- Split building of dependencies and package checking into different
  jobs which allows us to use 2x50 minutes. The check job uses the cache
  from the dependency job.
- Automatically deploy of pkgdown based documentation to github pages
  (a *P*ersonal *A*ccess *T*oken needs to be created and set in travis
  settings).
@lgatto
Copy link
Owner

lgatto commented Oct 19, 2018

Great, thanks @sgibb. I have created a PAT and added it to the travis setting.
I wouldn't mind 5 jobs, if you are happy to do it and think it's worth it.

I would prefer to keep all tests, as much as possible.

I will need to check if .isCentroidedFromFile is still needed, but migrating it to xml2 would be good anyway. But not sure about mzID - I think it is important to verify some id data validity against mzR But I indeed think that we could re-implement (nicer) MAplots to get rid of affy. We still need a dependency to plot the heatmap in the vignette though.

But yes, let's open a issue to keep track of the dependencies and try to reduce them.

@sgibb
Copy link
Collaborator Author

sgibb commented Oct 23, 2018

I added skip_on_travis to tests/testthat/test_MSmap.R. Somehow AnnotationHub could not download the file (even when the VM on travis has internet access). Sometimes it is working, e.g.: https://travis-ci.org/sgibb/MSnbase/builds/444428390
But I rerun this last job multiple times without a single success (we just need a single successful download because the ~/.AnnotationHub is cached, too).

You can simply revert it with git revert 7b7c32d6293e13d281c85e18b7867df0386f46fc.

IMHO this could be merged to master now.

Please note: I recognized that pkgdown::build_site fails because of:

<simpleError in add_classes_to_exports(ns = nsenv, package = package, exports = exports,     nsInfo = nsInfo): in ‘MSnbase’ methods for export not found: reduce>
There were 50 or more warnings (use warnings() to see the first 50)
Error in add_classes_to_exports(ns = nsenv, package = package, exports = exports,  : 
  in ‘MSnbase’ methods for export not found: reduce

I am not sure how to solve this (without exporting reduce, seems to be related to pkgload

@codecov-io
Copy link

codecov-io commented Oct 23, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@1c5f723). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #368   +/-   ##
=========================================
  Coverage          ?   70.94%           
=========================================
  Files             ?       82           
  Lines             ?     8188           
  Branches          ?        0           
=========================================
  Hits              ?     5809           
  Misses            ?     2379           
  Partials          ?        0

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 1c5f723...48bf02f. Read the comment docs.

@lgatto lgatto merged commit 315e52a into master Oct 24, 2018
@lgatto
Copy link
Owner

lgatto commented Oct 24, 2018

Thanks!

@lgatto
Copy link
Owner

lgatto commented Oct 26, 2018

@sgibb - is the pkgdown site build and deployed automatically, in the end?

@sgibb
Copy link
Collaborator Author

sgibb commented Oct 26, 2018

If the build of the master branch is successful, travis updates the gh-pages branch automatically (it works: https://github.com/lgatto/MSnbase/tree/gh-pages). But I guess you use the doc/ directory instead the gh-pages branch for the pkgdown site. So currently the automatically build site isn't visible.

There are two solutions:

  1. You use the gh-pages branch for the github pages website, remove the doc/ directory from master (that would also remove the need for branch-specific .gitignore files).
  2. Or I set the target_branch in .travis.yml to master but I was afraid that it could destroy our git history.

For https://github.com/sgibb/topdownr/ I use the first option.

@lgatto
Copy link
Owner

lgatto commented Oct 26, 2018

OK, thanks. I'll think about migrating to solution 1 above.

@lgatto
Copy link
Owner

lgatto commented Oct 30, 2018

I think the automatic deploy to gh-pages is a nice option. I modified the settings to use the gh-pages branch, but http://lgatto.github.io/MSnbase/ throws a 404 error. Not sure if it's only a matter of time, or did I miss something, @sgibb.

@lgatto
Copy link
Owner

lgatto commented Oct 30, 2018

All good now, it is live. I will apply the same strategy for my other packages. Thanks again @sgibb!

@sgibb
Copy link
Collaborator Author

sgibb commented Oct 30, 2018

I can't see a 404 but the regular pkgdown site. I guess it was just a matter of waiting for the next push from travis.

@sgibb sgibb deleted the travis branch January 24, 2019 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants