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

CI pipeline improvements (more label control) #7206

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

mbien
Copy link
Member

@mbien mbien commented Mar 28, 2024

A few tweaks in anticipation to the policy changes:

https://infra.apache.org/github-actions-policy.html

restructuring:

  • refactored the env vars into 'outputs' of the base-build job since env vars can't be used everywhere but outputs can
  • more jobs are now opt-in and can be activated with labels
    (enterprise, db, javafx, profiler, versioning, web...)
  • moved web related steps into the webcommon job to shrink ide job
  • activate LSP tests also when 'Gradle' or 'Maven' label was set
  • matrix got further reduced for runs outside of PRs
  • combine some jobs into single build-system / misc job

other changes:

  • fail CI if PR is not labeled
  • fixed issue where paperwork job couldn't be cancelled
  • bumped CV tests to JDK 22

full set of variables (or see labels with [ci] marker in their description):

# labels are mapped to env vars for pipeline customization. If this is not a PR, (almost) everything will run, but with a reduced matrix.
# note: env vars don't work in the job's 'if' field but do work within jobs ( https://github.com/actions/runner/issues/1189 ), the whole expression must be duplicated
# labels for special commands:
# 'ci:all-tests' enables everything
# 'ci:no-build' disables the build job (and test jobs too)
# 'ci:dev-build' produces an artifact containing a runnable NetBeans zip distribution
# 'Java' label
test_java: ${{ contains(github.event.pull_request.labels.*.name, 'Java') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'JavaFX' label
test_javafx: ${{ contains(github.event.pull_request.labels.*.name, 'JavaFX') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'JavaDoc' or 'API Change' labels
test_javadoc: ${{ contains(github.event.pull_request.labels.*.name, 'JavaDoc') || contains(github.event.pull_request.labels.*.name, 'API Change') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'JavaScript' label
test_javascript: ${{ contains(github.event.pull_request.labels.*.name, 'JavaScript') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'PHP' label
test_php: ${{ contains(github.event.pull_request.labels.*.name, 'PHP') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'Groovy' label
test_groovy: ${{ contains(github.event.pull_request.labels.*.name, 'Groovy') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'Rust' label
test_rust: ${{ contains(github.event.pull_request.labels.*.name, 'Rust') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'Platform' label
test_platform: ${{ contains(github.event.pull_request.labels.*.name, 'Platform') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'LSP' label for enabling Language Server Protocol tests
# 'Gradle' or 'Maven' will activate lsp tests too due to test dependencies on project API (ProjectViewTest, LspBrokenReferencesImplTest, ...)
test_lsp: ${{ contains(github.event.pull_request.labels.*.name, 'LSP') || contains(github.event.pull_request.labels.*.name, 'Gradle') || contains(github.event.pull_request.labels.*.name, 'Maven') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'GraalVM' label for tests requirering GraalVM
test_graalvm: ${{ contains(github.event.pull_request.labels.*.name, 'GraalVM') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'VSCode Extension' label for building and testing the VSCode Extension
test_vscode_extension: ${{ contains(github.event.pull_request.labels.*.name, 'VSCode Extension') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'Ant', 'Gradle', 'Maven' and 'MX' labels trigger the build-tools job
test_build_tools: ${{ contains(github.event.pull_request.labels.*.name, 'Ant') || contains(github.event.pull_request.labels.*.name, 'Gradle') || contains(github.event.pull_request.labels.*.name, 'Maven') || contains(github.event.pull_request.labels.*.name, 'MX') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'git', 'subversion' and 'mercurial' labels trigger the versioning job
test_versioning: ${{ contains(github.event.pull_request.labels.*.name, 'git') || contains(github.event.pull_request.labels.*.name, 'subversion') || contains(github.event.pull_request.labels.*.name, 'mercurial') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'Java EE/Jakarta EE', 'Micronaut' and 'enterprise' labels trigger the enterprise job
test_enterprise: ${{ contains(github.event.pull_request.labels.*.name, 'Java EE/Jakarta EE') || contains(github.event.pull_request.labels.*.name, 'Micronaut') || contains(github.event.pull_request.labels.*.name, 'enterprise') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'JavaScript', 'TypeScript', 'HTML', 'CSS', 'CSL' and 'web' labels the trigger web job
test_web: ${{ contains(github.event.pull_request.labels.*.name, 'JavaScript') || contains(github.event.pull_request.labels.*.name, 'TypeScript') || contains(github.event.pull_request.labels.*.name, 'HTML') || contains(github.event.pull_request.labels.*.name, 'CSS') || contains(github.event.pull_request.labels.*.name, 'CSL') || contains(github.event.pull_request.labels.*.name, 'web') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'tests' label activates an extra step which builds all tests
test_tests: ${{ contains(github.event.pull_request.labels.*.name, 'tests') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}

@mbien mbien added the CI continuous integration changes label Mar 28, 2024
@mbien mbien added this to the NB22 milestone Mar 28, 2024
@mbien mbien force-pushed the ci-tweaks-for-new-policy branch from 80b4e3a to 7d4fdf3 Compare March 28, 2024 19:08
@mbien

This comment was marked as outdated.

@mbien mbien force-pushed the ci-tweaks-for-new-policy branch 2 times, most recently from 19a2a19 to ccfd8da Compare March 28, 2024 22:58
Comment on lines 102 to 113
# 'git', 'subversion' and 'mercurial' labels trigger versioning job
test_versioning: ${{ contains(github.event.pull_request.labels.*.name, 'git') || contains(github.event.pull_request.labels.*.name, 'subversion') || contains(github.event.pull_request.labels.*.name, 'mercurial') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}

# 'Java EE/Jakarta EE', 'Micronaut' and 'enterprise' labels trigger enterprise job
test_enterprise: ${{ contains(github.event.pull_request.labels.*.name, 'Java EE/Jakarta EE') || contains(github.event.pull_request.labels.*.name, 'Micronaut') || contains(github.event.pull_request.labels.*.name, 'enterprise') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}

# 'JavaScript', 'TypeScript', 'HTML', 'CSS', 'CSL' and 'web' labels trigger enterprise job
test_web: ${{ contains(github.event.pull_request.labels.*.name, 'JavaScript') || contains(github.event.pull_request.labels.*.name, 'TypeScript') || contains(github.event.pull_request.labels.*.name, 'HTML') || contains(github.event.pull_request.labels.*.name, 'CSS') || contains(github.event.pull_request.labels.*.name, 'CSL') || contains(github.event.pull_request.labels.*.name, 'web') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}

# 'tests' label activates an extra step which builds all tests
test_tests: ${{ contains(github.event.pull_request.labels.*.name, 'tests') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
Copy link
Member Author

@mbien mbien Mar 31, 2024

Choose a reason for hiding this comment

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

@sdedic since I believe you asked me about this before: this would change the enterprise job to be also opt-in. It listens to enterprise [ci] enable enterprise job , Java EE/Jakarta EE [ci] enable enterprise job and Micronaut [ci] enable enterprise job after this change. Any other labels to add?

analog to the build-tools, web and versioning jobs

@mbien mbien force-pushed the ci-tweaks-for-new-policy branch 4 times, most recently from 43e4c1a to 0ce7358 Compare April 12, 2024 19:24
Comment on lines +303 to +298
- name: Check PR labels
if: ${{ github.event_name == 'pull_request' && join(github.event.pull_request.labels.*.name) == '' }}
run: |
echo "::error::PRs must be labeled, see: https://cwiki.apache.org/confluence/display/NETBEANS/PRs+and+You+-+A+reviewer+Guide"
exit 1
Copy link
Member Author

Choose a reason for hiding this comment

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

this will fail when PRs are not labeled.

CI did always run some tests outside of the declared categories. The reasoning behind that was because I was worried about unlabeled PRs. Since this PR reduces the amount of testing outside of the declared labels, it will be enforced from now on (was discussed on apache slack with @neilcsmith-net ).

It should be easy to find labels even for trivial PRs, since we have enough labels which are generic and don't do anything. E.g Editor , CI continuous integration changes or Code cleanup . For readme edits we have ci:no-build [ci] disable CI pipeline

The IDE cluster job is always enabled, one day we should make it also configurable but lets do this incrementally.

@mbien mbien marked this pull request as ready for review April 12, 2024 19:37
@mbien mbien force-pushed the ci-tweaks-for-new-policy branch from 0ce7358 to 557ebb8 Compare April 14, 2024 19:07
Comment on lines 90 to +92
# 'LSP' label for enabling Language Server Protocol tests
test_lsp: ${{ contains(github.event.pull_request.labels.*.name, 'LSP') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
# 'Gradle' or 'Maven' will activate lsp tests too due to test dependencies on project API (ProjectViewTest, LspBrokenReferencesImplTest, ...)
test_lsp: ${{ contains(github.event.pull_request.labels.*.name, 'LSP') || contains(github.event.pull_request.labels.*.name, 'Gradle') || contains(github.event.pull_request.labels.*.name, 'Maven') || contains(github.event.pull_request.labels.*.name, 'ci:all-tests') || github.event_name != 'pull_request' }}
Copy link
Member Author

@mbien mbien Apr 14, 2024

Choose a reason for hiding this comment

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

lsp job is now also active with Gradle [ci] enable "build tools" tests or Maven [ci] enable "build tools" tests due to test dependencies on project API.

This was added to avoid issues like #7267 in future. Anything else to change?

@lkishalmi @sdedic @neilcsmith-net

@mbien mbien added ci:all-tests [ci] enable all tests CI continuous integration changes and removed CI continuous integration changes labels Apr 17, 2024
@mbien mbien force-pushed the ci-tweaks-for-new-policy branch from 557ebb8 to 3ac180f Compare April 17, 2024 03:17
@mbien mbien removed the ci:all-tests [ci] enable all tests label Apr 17, 2024
@mbien
Copy link
Member Author

mbien commented Apr 17, 2024

ran it once with all-all tests and it looks fine, CV on JDK 22 fails since it requires #7268

@mbien mbien added the ci:all-tests [ci] enable all tests label Apr 17, 2024
A few tweaks in anticipation to the policy changes:

https://infra.apache.org/github-actions-policy.html

restructuring:

 - more jobs are now opt-in and can be activated with labels
   (enterprise, db, javafx, profiler, versioning, web...)
 - moved web related steps into the webcommon job to shrink ide job
 - activate LSP tests also when 'Gradle' or 'Maven' label was set
 - matrix got further reduced for runs outside of PRs
 - combine some jobs into single build-system / misc job

other changes:

 - fail CI if PR is not labeled
 - fixed issue where paperwork job couldn't be cancelled
 - bumped CV tests to JDK 22
@mbien mbien force-pushed the ci-tweaks-for-new-policy branch from 3ac180f to f3fa61b Compare April 17, 2024 15:40
@mbien mbien merged commit 1a4519c into apache:master Apr 17, 2024
41 checks passed
Comment on lines 125 to +129
strategy:
matrix:
java: [ '17', '21', '22' ]
exclude:
- java: ${{ github.event_name == 'pull_request' && 'nothing' || '21' }}
Copy link
Member Author

Choose a reason for hiding this comment

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

for future reference: this trick allows us to reduce the test matrix using labels or other properties

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:all-tests [ci] enable all tests CI continuous integration changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant