Setup GitHub Actions CI and SNAPSHOT uploading #440

lazaroclapp · 2021-01-04T19:44:15Z

This change sets up GitHub Actions as a parallel build CI to Travis (for now). GitHub Actions should build NullAway on MacOS X and Linux using JDK 8, and on Linux using JDK 11, mirroring our existing Travis config.

Additionally, this migrates snapshot uploading and coverage tracking to GitHub Actions, disabling the equivalent Travis CI events as they will likely conflict. This might break snapshot uploading after landing, since it's not possible to easily test that from the PR. If it does, follow up PRs might be needed.

See #408

lazaroclapp · 2021-01-04T19:44:32Z

Do not merge yet. WIP. Testing.

…PR creation implies a push)

lazaroclapp · 2021-01-04T20:10:05Z

@msridhar No idea if Snapshot publishing works yet, but GitHub action CI builds themselves look sane :)

msridhar

Overall looks great! Just a couple non-blocking comments

msridhar · 2021-01-04T20:57:34Z

.github/workflows/continuous-integration.yml

+        run: ./gradlew :nullaway:test
+        if: matrix.java == '11'
+      - name: Report jacoco coverage
+        run: ./gradlew jacocoTestReport coveralls


In the logs I see for coveralls:

> Task :nullaway:coveralls no available CI service

Is that ok for now?

Ah, let me look into that a bit. Might be ok, but we are not in a rush.

Since the author of kt3k/coveralls-gradle-plugin is looking for a new maintainer (see kt3k/coveralls-gradle-plugin#107) and nbaztec/coveralls-jacoco-gradle-plugin supports GitHub Actions out-of-the-box, it might be worth migrating to the latter.

https://github.com/nbaztec/coveralls-jacoco-gradle-plugin#github-actions

msridhar · 2021-01-04T20:58:16Z

.github/workflows/continuous-integration.yml

+  publish_snapshot:
+    name: 'Publish snapshot'
+    needs: [build]
+    if: github.event_name == 'push' && github.repository == 'uber/NullAway' && github.ref == 'refs/heads/master'


You could test the snapshot upload out by changing the branch here temporarily, and then changing it back. I'm also ok with just testing on master.

msridhar · 2021-01-04T21:01:32Z

.github/workflows/continuous-integration.yml

+          java-version: 8
+      - name: 'Publish'
+        env:
+          SONATYPE_NEXUS_USERNAME: ${{ secrets.SONATYPE_NEXUS_USERNAME }}


For WALA I had to do this to get env vars to show up as Gradle properties in the build:

ORG_GRADLE_PROJECT_SONATYPE_NEXUS_USERNAME: ${{ secrets.SONATYPE_NEXUS_USERNAME }} ORG_GRADLE_PROJECT_SONATYPE_NEXUS_PASSWORD: ${{ secrets.SONATYPE_NEXUS_PASSWORD }}

But the WALA Gradle scripts are different from NullAway's so maybe this is correct here.

Our scripts look for SONATYPE_NEXUS_USERNAME, etc, in env vars. So I suspect this will work, but let me test it.

Actually, you were right, ORG_GRADLE_PROJECT_SONATYPE_NEXUS_USERNAME works for some reason and SONATYPE_NEXUS_USERNAME doesn't. I could check out why, but since it seems to be working now, I'll just update the branch and land.

It's because the Gradle scripts are looking for a project property SONATYPE_NEXUS_USERNAME, etc., and you need the ORG_GRADLE_PROJECT_ prefix to have Gradle read the environment variable as a project property: https://docs.gradle.org/current/userguide/build_environment.html#sec:project_properties

msridhar · 2021-01-04T21:02:25Z

BTW, I was a little surprised this "just worked" without doing anything explicit to set up the Android SDK in the builds. Looks like Android comes pre-installed on the GH Actions runners; nice!

lazaroclapp · 2021-01-04T21:05:53Z

BTW, I was a little surprised this "just worked" without doing anything explicit to set up the Android SDK in the builds. Looks like Android comes pre-installed on the GH Actions runners; nice!

Yup. I had to read into that. Was similarly confused for a while, could have probably saved you the time in searching, though 😅

msridhar · 2021-01-04T22:15:30Z

BTW does each PR creation really imply a push? What if it's a PR from someone without push access to this repo?

lazaroclapp · 2021-01-04T22:56:18Z

BTW does each PR creation really imply a push? What if it's a PR from someone without push access to this repo?

Not sure. I guess the only way to test it is to try to create a PR from a fork or some other repo. The problem with pull_request (as configured in WALA) is that it will only trigger at PR creation and it will then duplicate what we see from just push (so you get twice as many jobs in the PR). There is a way to also trigger when the PR changes, but again that will cause these jobs to show twice... I'd want to test if nothing shows right now for a PR from a different repo before enabling that much redundancy.

Might experiment with that tomorrow (also, there seems to be still a step missing in uploading coverage results :) )

joschi · 2021-01-05T07:26:39Z

.github/workflows/continuous-integration.yml

+      - name: Cache Gradle caches
+        uses: actions/cache@v1
+        with:
+          path: ~/.gradle/caches
+          key: ${{ runner.os }}-gradle-caches-${{ hashFiles('**/*.gradle') }}
+          restore-keys: ${{ runner.os }}-gradle-caches-
+      - name: Cache Gradle wrapper
+        uses: actions/cache@v1
+        with:
+          path: ~/.gradle/wrapper
+          key: ${{ runner.os }}-gradlew-wrapper-${{ hashFiles('gradle/wrapper/gradle-wrapper.properties') }}
+          restore-keys: ${{ runner.os }}-gradlew-wrapper-


Using eskatos/gradle-command-action might reduce the boilerplate necessary to set up proper caching for Gradle builds.

https://github.com/eskatos is working for Gradle, so it's not some random 3rd party GitHub action.

Makes sense to me. Thanks! :)

lazaroclapp · 2021-01-05T17:51:08Z

jar-infer/nullaway-integration-test/build.gradle

@@ -15,9 +15,6 @@
 */
 plugins {
  id "java"
-  // For code coverage:
-  id 'jacoco'
-  id 'com.github.kt3k.coveralls'


Coverage plugin has multi-project support. But:
a) It's a bit convoluted
b) We have never had proper coverage support for JarInfer before (this bit above is long broken)
c) We mostly care about coverage for core NullAway

That said, it's worth having JarInfer coverage in the future as well, just not high priority.

lazaroclapp · 2021-01-05T19:15:52Z

Coveralls plug-in was sometimes failing with a timeout. Not sure if this will keep happening after merging or if the last change I made truly fixed it (ran it twice, successfully), but I'd say let's keep an eye on it and fix as needed.

Also, not sure if the GitHub Actions CI will trigger for third-party PRs. Since we are running both GitHub Actions and Travis CI for the time being, I suggest we just keep an eye out for anything that seems to be skipping GitHub Actions.

Will merge unless there are concerns from @msridhar

msridhar · 2021-01-05T19:18:11Z

LGTM; I'm fine with landing. My feeling is that CI won't run for 3rd-party PRs right now, but we can easily test that once this lands

lazaroclapp added 2 commits January 4, 2021 14:30

Setup GitHub Actions CI and SNAPSHOT uploading.

b2e12f3

Remove potentially conflicting Travis CI post-build changes.

43c8a56

lazaroclapp added 3 commits January 4, 2021 14:53

Fix typos. Reduce duplicated jobs.

80187fd

Trigger on push for all branches, but not again on PR creation (each …

ea3f82d

…PR creation implies a push)

Typos

4cf8e7c

lazaroclapp requested a review from msridhar January 4, 2021 20:09

msridhar approved these changes Jan 4, 2021

View reviewed changes

lazaroclapp added 4 commits January 4, 2021 16:28

Upgrade Coveralls for GitHub Actions compatibility

915e71c

Temporary force SNAPSHOT release from outside master

b5eab42

Try prefix for SONATYPE secrets.

f556ee5

SNAPSHOTS back to master branch.

bbf5e03

joschi reviewed Jan 5, 2021

View reviewed changes

Switch coverage plug-in and simplify GH Actions gradlew config.

6607c96

lazaroclapp commented Jan 5, 2021

View reviewed changes

lazaroclapp added 4 commits January 5, 2021 13:06

Upgrade Android SDK and build-tools

4e316bb

Repair Travis CI scripts (for now).

a10762a

Try test on Windows and JDK11+Mac

4a1966f

Attempt to fix timeout

589d035

lazaroclapp merged commit eed3fc0 into master Jan 5, 2021

lazaroclapp deleted the lazaro_ci_to_github_actions branch January 5, 2021 19:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup GitHub Actions CI and SNAPSHOT uploading #440

Setup GitHub Actions CI and SNAPSHOT uploading #440

lazaroclapp commented Jan 4, 2021

lazaroclapp commented Jan 4, 2021

lazaroclapp commented Jan 4, 2021

msridhar left a comment

msridhar Jan 4, 2021

lazaroclapp Jan 4, 2021

joschi Jan 5, 2021

msridhar Jan 4, 2021

msridhar Jan 4, 2021

lazaroclapp Jan 4, 2021

lazaroclapp Jan 4, 2021

msridhar Jan 4, 2021

msridhar commented Jan 4, 2021

lazaroclapp commented Jan 4, 2021

msridhar commented Jan 4, 2021

lazaroclapp commented Jan 4, 2021 •

edited

Loading

joschi Jan 5, 2021

lazaroclapp Jan 5, 2021

lazaroclapp Jan 5, 2021

lazaroclapp commented Jan 5, 2021

msridhar commented Jan 5, 2021

Setup GitHub Actions CI and SNAPSHOT uploading #440

Setup GitHub Actions CI and SNAPSHOT uploading #440

Conversation

lazaroclapp commented Jan 4, 2021

lazaroclapp commented Jan 4, 2021

lazaroclapp commented Jan 4, 2021

msridhar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msridhar commented Jan 4, 2021

lazaroclapp commented Jan 4, 2021

msridhar commented Jan 4, 2021

lazaroclapp commented Jan 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lazaroclapp commented Jan 5, 2021

msridhar commented Jan 5, 2021

lazaroclapp commented Jan 4, 2021 •

edited

Loading