Skip to content

Taskcluster: add dev build and unit tests on macOS #22185

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Nov 15, 2018
Merged

Taskcluster: add dev build and unit tests on macOS #22185

merged 10 commits into from
Nov 15, 2018

Conversation

SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Nov 12, 2018 •
edited
Loading

For simplicity, Salt is used in agentless mode for now.

I’m also hoping to have most of the dependencies be installed by tasks for themselves, so that changes can be made in the same servo/servo PR that uses them without redeploying servers.

This change is Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 12, 2018
Manishearth
Manishearth approved these changes Nov 12, 2018
View reviewed changes
etc/taskcluster/macos/Saltfile Outdated
@@ -0,0 +1,2 @@
salt-ssh:
config_dir: ./config
Copy link
Member

Choose a reason for hiding this comment

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

missing newlines at eof for most of these files

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

etc/taskcluster/macos/states/generic-worker.sls Outdated
file.managed:
- mode: 644
- template: jinja
- contents: >-
Copy link
Member

Choose a reason for hiding this comment

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

is it possible to store this file separately?

i suppose the substs make this harder

nah, we use files with substs in our current salt setup: https://github.com/servo/saltfs/blob/master/homu/init.sls#L69

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

etc/taskcluster/macos/states/generic-worker.sls Outdated
- contents: |-
#!/bin/sh
# generic-worker overwrites its config file to fill in defaults,
# but we want to avoid touching config.json here
Copy link
Member

Choose a reason for hiding this comment

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

try to split this into its own file if possible

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed as part of another change.

jdm
jdm reviewed Nov 13, 2018
View reviewed changes
etc/taskcluster/macos/README.md

* Add a correponding entry in the `config/roster` file.

* Log in through VNC, and run `xcode-select --install`
Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/servo/servo/wiki/SaltStack-Administration has a number of other steps we follow right now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m hoping to make some of those taken care of by Salt or by tasks, but installing XCode requires GUI interaction.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, this document is intended to expand over time as we do more, it is not at an end state. This is documenting what I’ve done so far and how to reproduce it.

@SimonSapin SimonSapin changed the title Add SaltStack configuration for macOS Taskcluster workers Taskcluster: add dev build and unit tests on macOS Nov 15, 2018
@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #22201) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Nov 15, 2018
Manishearth
Manishearth approved these changes Nov 15, 2018
View reviewed changes
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

r=me

@SimonSapin
Copy link
Member Author

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

📌 Commit 1c7dbe1 has been approved by Manishearth

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-rebase There are merge conflict errors. labels Nov 15, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 1c7dbe1 with merge b8281b4...

bors-servo pushed a commit that referenced this pull request Nov 15, 2018
Auto merge of #22185 - servo:tc-mac, r=Manishearth
b8281b4 
Taskcluster: add dev build and unit tests on macOS

For simplicity, Salt is used in [agentless](https://docs.saltstack.com/en/getstarted/ssh/) mode for now.

I’m also hoping to have most of the dependencies be installed by tasks for themselves, so that changes can be made in the same `servo/servo` PR that uses them without redeploying servers.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22185)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - mac-rel-css2

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Nov 15, 2018
@SimonSapin
Copy link
Member Author 

  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/unloading-documents/unload/004.html:
  │ FAIL [expected PASS] pagehide / unload event order
  │   → assert_array_equals: lengths differ, expected 2 got 3
  │ 
  │ onload/</iframe.onload/iframe.onload<@http://web-platform.test:8000/html/browsers/browsing-the-web/unloading-documents/unload/004.html:19:51
  │ Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1568:20
  └ Test.prototype.step_func/<@http://web-platform.test:8000/resources/testharness.js:1592:20

@bors-servo retry

@bors-servo
Copy link
Contributor

⚡ Previous build results for android, android-mac, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, status-taskcluster, windows-msvc-dev are reusable. Rebuilding only mac-rel-css1, mac-rel-css2...

@bors-servo
Copy link
Contributor

☀️ Test successful - android, android-mac, android-x86, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, status-taskcluster, windows-msvc-dev
Approved by: Manishearth
Pushing b8281b4 to master...

@bors-servo bors-servo merged commit 1c7dbe1 into master Nov 15, 2018
@SimonSapin SimonSapin deleted the tc-mac branch November 15, 2018 20:24
@SimonSapin SimonSapin mentioned this pull request Nov 30, 2018
Closed
31 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm jdm left review comments

@Manishearth Manishearth Manishearth approved these changes

Assignees

@Manishearth Manishearth

Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SimonSapin @bors-servo @jdm @Manishearth @highfive