Skip to content
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

Add R build test to CircleCI #1238

Merged
merged 29 commits into from
May 22, 2020
Merged

Add R build test to CircleCI #1238

merged 29 commits into from
May 22, 2020

Conversation

rpkyle
Copy link
Contributor

@rpkyle rpkyle commented May 9, 2020

This PR proposes a set of tests to assess the health of the entire Dash component library build pipeline for R, as we currently do with Dash for Python.

There have been a few instances of package builds in the past using dash-generate-components which appeared to succeed, but failed to install or work properly due to minor misconfiguration issues. A build test would help identify these inconsistencies prior to a Dash release.

Late last year, @Marc-Andre-Rivet described a desirable workflow for testing R builds upon pushing to dev:

  • trigger a rebuild of Dash for R within CircleCI, and all Dash Core packages using the versions at the head of their respective dev branches
  • install the rebuilt packages within CircleCI
  • launch unit tests to assess basic package functionality in R
  • then, if unit tests are successful, runs a basic integration test to demonstrate that the dashr-on-premise-sample-app is functional and interactive (such a test is proposed in Add dopsa test for proposed R build test dashR#199)

I suggest adding the following integrity checks to the above list:

  • remove non-R artifacts from each of the cloned repositories (these can cause errors/warnings and lengthen the time required to tarball/check packages)
  • run R CMD build to ensure that packages are properly formatted, can be built into tarballs, and are ready for R CMD check
  • run R CMD check --as-cran --no-manual to ensure that changes have not violated CRAN requirements

The current draft appears to work generally as intended.

@Marc-Andre-Rivet @alexcjohnson @josegonzalez

rpkyle and others added 6 commits May 9, 2020 02:48
* wrap descriptions at 60 chars

* fix wildcard handling

* support for non-CSS/JS deps in assets

* add value tag for .Rd files

* add copyright field to DESCRIPTION

* add Authors@R line to DESCRIPTION

* fix --r-suggests lstrip bug

* +sp after , if missing in R imp/sugg/deps
@rpkyle rpkyle marked this pull request as ready for review May 13, 2020 05:35
@rpkyle
Copy link
Contributor Author

rpkyle commented May 13, 2020

@alexcjohnson @Marc-Andre-Rivet I'm struggling to understand I have resolved the remaining test failures, but I think this is ready for review!

.circleci/config.yml Outdated Show resolved Hide resolved
@rpkyle rpkyle requested a review from alexcjohnson May 19, 2020 04:23
git clone --depth 1 https://github.com/plotly/dashR.git -b dev dashR
git clone --depth 1 https://github.com/plotly/dash-html-components.git -b dev
git clone --depth 1 https://github.com/plotly/dash-core-components.git -b dev
git clone --depth 1 https://github.com/plotly/dash-table.git -b dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for -b dev as the default branch is dev for html/dcc/table

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 74083ce

@alexcjohnson
Copy link
Collaborator

OK there's something very odd going on with our percy config - looking at a passing percy build from another branch we have 7 builds for the same commit, with only the last one containing the snapshots. Apparently it all looks OK since the last one passes but it's a mess, should be only one.
Screen Shot 2020-05-21 at 9 50 50 PM
Whereas on this branch there's one more empty build, and now it comes after the real build so now it all fails?
Screen Shot 2020-05-21 at 9 53 56 PM

@alexcjohnson
Copy link
Collaborator

dcc has a similar test job structure and only has one percy build per commit. The only difference I see is the env var PERCY_PARALLEL_TOTAL set in both of the snapshot-generating jobs:

        environment:
            PYTHON_VERSION: py37
            PERCY_PARALLEL_TOTAL: -1

@rpkyle
Copy link
Contributor Author

rpkyle commented May 22, 2020

dcc has a similar test job structure and only has one percy build per commit. The only difference I see is the env var PERCY_PARALLEL_TOTAL set in both of the snapshot-generating jobs:

        environment:
            PYTHON_VERSION: py37
            PERCY_PARALLEL_TOTAL: -1

Apparently this tells Percy we're not sure how many parallel threads this run will have, but we're done when the orb sends finalize_all.

I'm not sure what that means, since whenever I hear "orb" I think Little Fluffy Clouds. Should we modify this setting? Is Percy not reporting the finalized state consistently?

@rpkyle
Copy link
Contributor Author

rpkyle commented May 22, 2020

I don't understand why Percy doesn't think it's finalized... something about the parallel jobs confusing it?

Seems possible, could it be related to our parallelism setting also? Setting PERCY_PARALLEL_TOTAL: -1 seems to have fixed the issue with the R test, though there are still some empty builds occurring -- we should investigate these at some point.

@rpkyle rpkyle merged commit 316b662 into dev May 22, 2020
@rpkyle rpkyle deleted the add-rbuild-test branch May 22, 2020 14:29
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