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] Disable macOS build on PRs #19754

Closed
kuisathaverat opened this issue Jul 8, 2020 · 5 comments · Fixed by #20069
Closed

[CI] Disable macOS build on PRs #19754

kuisathaverat opened this issue Jul 8, 2020 · 5 comments · Fixed by #20069
Assignees
Labels
automation ci enhancement Team:Automation Label for the Observability productivity team

Comments

@kuisathaverat
Copy link
Contributor

kuisathaverat commented Jul 8, 2020

Due we have limited macOS resources we will keep then bot the branch merges, and other test. Thus we going to disable the macOS test by default on PRs. However will be possible to execute those test manually, with a comment, or adding a label to the PR.

Manually

 Scenario: A developer creates a new PR
    When a developer Launch the beats pipeline from Jenkins UI with the parameter macosTest
    Then a build is triggered  
    And the macOS tests are executed

NOTE: Only users with permissions in the CI will be able to login and trigger the build manually using the UI.

GH Label

 Scenario: A developer creates a new PR
    When a Beat requires macOS test
    Then the test is skipped if the build is not labeled as macOS test needed (label macOS)

NOTE: Only users with write permissions in the repo can add labels to the PR. In addition, builds won't be triggered automatically in the even of a label change.

GH Comment

 Scenario: A developer creates a new PR
    When a developer put the GitHub comment `/test mac` comment 
    Then a build is triggered  
    And the macOS tests are executed

cc @elastic/observablt-robots

@v1v
Copy link
Member

v1v commented Jul 21, 2020

I like this approach and even more the well described scenarios 💯 !!

Regarding the GH comment, I don't know what's the current standardisation in the beats project, but in other projects we use jenkins run the tests, so I'd say to use jenkins run the tests for macos if possible to keep the backward compatibility with the current implementation in production. thoughts?

@cachedout
Copy link
Contributor

I personally think that we should split it up like this:

Comments should be about actions: jenkins run the tests but labels should be about what tests to run. (OSX, etc). That means that a developer only ever has to remember a single "command" to initiate a test run and she can also easily reason about what options might be available by browsing through the labels.

WDYT?

@v1v
Copy link
Member

v1v commented Jul 21, 2020

Maybe if we provide such amount of flexibility it might cause some misunderstanding, that's for sure

I'm not really familiar with the PR review process for the beats project, but I'd say those scenarios could be valid since they are not exclusive with each other. Let me pincel a few scenarios with how those actions can enhance the build system.

Static behaviour.

  • Contributor creates the PR.
  • Reviewer find the OS darwin should be enabled by default, by adding the label macOS
  • Contributor keeps committing new changes, the build will always validate the macOS stages.

Dynamic behaviour

  • Contributor creates the PR.
  • Reviewer is not sure if the PR will require the validation for the macOS, he forces the build with jenkins run the tests for macos
    • Reviewer finds there is a nonfunctional dependency with macOS and sets the label `macOS
    • Reviewer doesn't find any dependency with the macOS then he does nothing.

Maybe we can ask the beats teams about the above use cases?

@v1v v1v self-assigned this Jul 21, 2020
@cachedout
Copy link
Contributor

A point I've just thought of is that I'm not sure if contributors have rights to add labels in the Beats project. If not, that would mean that my proposal wouldn't work.

@v1v
Copy link
Member

v1v commented Jul 22, 2020

A point I've just thought of is that I'm not sure if contributors have rights to add labels in the Beats project. If not, that would mean that my proposal wouldn't work.

Only Elasticians got write privileges to the repo. I just updated the above-mentioned scenarios in the description with this particular requirement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation ci enhancement Team:Automation Label for the Observability productivity team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants