Closed Bug 1274980 Opened 8 years ago Closed 8 years ago

Configure test tasks for Mac OSX builds

Categories

(Firefox Build System :: Task Configuration, task, P1)

All
macOS
task

Tracking

(Not tracked)

RESOLVED FIXED
mozilla52

People

(Reporter: wcosta, Assigned: wcosta)

References

Details

User Story

Add test Mac OSX test tasks for in tree task set.

Attachments

(3 files, 1 obsolete file)

No description provided.
initial work is to copy linux64 test definitions to see what runs! a few differences: 1) we don't run Ru (reftest unaccelerated) on osx 2) we don't run Jit (jittests*) on osx 3) mochitest-plain is 10 chunks on linux64, should be 5 on osx 4) mochitest-chrome is split out of mochitest-other runs as 3 chunks, should be 1 chunk on osx 5) reftests are run as 8 chunks on linux, should be 1 chunk on osx 6) xpcshell is run as 5 chunks on linux, should be 1 chunk on osx lets start with that until things are green, we might have a few other modifications to made depending on runtime, etc.
Assignee: nobody → wcosta
Status: NEW → ASSIGNED
Depends on: 1273981
it looks like all the jobs are properly defined, and clipboard and gpu jobs are green now :)
Depends on: 1197256
No longer depends on: 1197256
Depends on: 1294090
Blocks: 1298431
Summary: [taskcluster-worker] [macosx-engine] Configure test tasks for Mac OSX builds → Configure test tasks for Mac OSX builds
Attachment #8796221 - Flags: feedback?(jmaher)
Comment on attachment 8796219 [details] Bug 1274980 part 1: Add support for test platform regex match. https://reviewboard.mozilla.org/r/82118/#review80740 I'm guessing this was a rebase over the existing addition of this functionality, but the test case is good! ::: taskcluster/taskgraph/transforms/base.py:10 (Diff revision 1) > +import re > > - Please remove the diffs in this file -- it's redundantly importing re
Attachment #8796219 - Flags: review?(dustin) → review+
Comment on attachment 8796220 [details] Bug 1274980 part 2: Support Mac OS tests in tc intree config. https://reviewboard.mozilla.org/r/82120/#review80742 No need (I cherry-picked earlier versions of this and the parent commit into bug 1286086, which is why they're mostly empty now)
Attachment #8796220 - Flags: review?(dustin) → review-
Attachment #8796221 - Attachment is obsolete: true
Attachment #8796221 - Flags: review?(dustin)
Attachment #8796221 - Flags: feedback?(jmaher)
Attachment #8796220 - Flags: feedback?(jmaher)
Comment on attachment 8796221 [details] Bug 1274980 part 3: Support Mac OS tests in tc intree config. https://reviewboard.mozilla.org/r/82122/#review80744 A few minor issues, but I really like the looks of this! ::: taskcluster/ci/desktop-test/tests.yml:18 (Diff revision 1) > # Coming soon: > # win.*: > # - unittests/win_unittest.py > # - remove_executables.py > # ... please remove this comment -- it's here now! ::: taskcluster/taskgraph/transforms/task.py:261 (Diff revision 1) > + # relative expiration date > + 'expires': basestring I left expires off for the docker worker, defaulting it to the task expiration. If you have a use case for using different expirations, it makes sense to add it here, but it should be optional and the default when not supplied should be the task expiration ::: taskcluster/taskgraph/transforms/task.py:438 (Diff revision 1) > + artifacts = map(lambda artifact: { > + 'name': artifact['name'], > + 'path': artifact['path'], > + 'type': artifact['type'], > + 'expires': task_def['expires'] > + }, worker['artifacts']); Is this map actually changing anything? ::: taskcluster/taskgraph/transforms/tests/make_task_description.py:31 (Diff revision 1) > ARTIFACTS = [ > # (artifact name prefix, in-image path) > ("public/logs/", "/home/worker/workspace/build/upload/logs/"), > ("public/test", "/home/worker/artifacts/"), > ("public/test_info/", "/home/worker/workspace/build/blobber_upload_dir/"), > ] > > +ARTIFACTS_MAC = [ > + # (artifact name prefix, in-image path) > + ("public/logs/", "build/upload/logs"), > + ("public/test", "artifacts"), > + ("public/test_info/", "build/blobber_upload_dir"), > +] These are almost the same, with the difference of the `/home/worker` prefix. Could you do this with one list? ::: taskcluster/taskgraph/transforms/tests/make_task_description.py:159 (Diff revision 1) > worker['loopback-audio'] = test['loopback-audio'] > worker['max-run-time'] = test['max-run-time'] > > worker['artifacts'] = [{ > 'name': prefix, > - 'path': path, > + 'path': path, 'type': 'directory', oops? ::: taskcluster/taskgraph/transforms/tests/make_task_description.py:261 (Diff revision 1) > + # taskdesc['worker-type'] = { > + # 'default': 'aws-provisioner-v1/macosx-test', > + # 'large': 'aws-provisioner-v1/macosx-test-large', > + # 'xlarge': 'aws-provisioner-v1/macosx-test-xlarge', > + # }[test['instance-size']] I don't like committing commented-out code. Does this have to stick around? ::: taskcluster/taskgraph/transforms/tests/make_task_description.py:267 (Diff revision 1) > + # 'default': 'aws-provisioner-v1/macosx-test', > + # 'large': 'aws-provisioner-v1/macosx-test-large', > + # 'xlarge': 'aws-provisioner-v1/macosx-test-xlarge', > + # }[test['instance-size']] > + > + taskdesc['worker-type'] = 'tc-worker-provisioner/gecko-test-macosx64' I haven't heard of this new provisioner - I assume it's temporary? FWIW the workerType should be gecko-t-* ::: taskcluster/taskgraph/transforms/tests/make_task_description.py:275 (Diff revision 1) > + worker['implementation'] = test['worker-implementation'] > + > + worker['artifacts'] = [{ > + 'name': prefix.rstrip('/'), > + 'path': path.rstrip('/'), > + 'type': 'directory', This seems to omit "expires"? Is that intentional? ::: taskcluster/taskgraph/transforms/tests/test_description.py:170 (Diff revision 1) > # This mozharness script also runs in Buildbot and tries to read a > # buildbot config file, so tell it not to do so in TaskCluster > Required('no-read-buildbot-config', default=False): bool, > > + # The branch name for the binary upload server. > + Optional('blob-upload-branch'): bool, This comment is confusing -- how can a boolean be a branch name? How about calling it `include-blob-upload-branch` and indicating in the comments here that if true it will add the `--blob-upload-branch=<project>` argument?
Attachment #8796221 - Flags: review-
Comment on attachment 8796220 [details] Bug 1274980 part 2: Support Mac OS tests in tc intree config. https://reviewboard.mozilla.org/r/82120/#review80752 ::: taskcluster/ci/desktop-test/test-platforms.yml:38 (Diff revision 3) > test-set: jsdcov-code-coverage-tests > + > +macosx64/debug: > + build-platform: macosx64/debug > + test-set: macosx64-tests > +macosx64/opt: I would prefer to start with debug as we could realistically migrate them. ::: taskcluster/ci/desktop-test/tests.yml:63 (Diff revision 3) > script: firefox_media_tests_buildbot.py > no-read-buildbot-config: true > config: > - - mediatests/buildbot_posix_config.py > - remove_executables.py > + - mediatests/buildbot_posix_config.py why is this moved? just hacking around and leftover? ::: taskcluster/taskgraph/transforms/tests/desktop_test.py:27 (Diff revision 3) > def set_defaults(config, tests): > for test in tests: > - test['mozharness']['build-artifact-name'] = 'public/build/target.tar.bz2' > + test['mozharness']['build-artifact-name'] = 'public/build/' + \ > + ('target.dmg' > + if test['build-platform'].startswith('macos') > + else 'target.tar.bz2') this will get more complicated for windows- if we can if/elif/else, that is fine, otherwise possibly switch this to an old school structured if statement?
Comment on attachment 8796219 [details] Bug 1274980 part 1: Add support for test platform regex match. https://reviewboard.mozilla.org/r/82118/#review80764
Comment on attachment 8796219 [details] Bug 1274980 part 1: Add support for test platform regex match. https://reviewboard.mozilla.org/r/82118/#review80770 Actually, you should probably change the commit message here, too! Anyway, no need to re-review that.
Comment on attachment 8796220 [details] Bug 1274980 part 2: Support Mac OS tests in tc intree config. https://reviewboard.mozilla.org/r/82120/#review80774
Attachment #8796220 - Flags: review-
Attachment #8796220 - Flags: feedback?(jmaher) → feedback-
Comment on attachment 8796220 [details] Bug 1274980 part 2: Support Mac OS tests in tc intree config. https://reviewboard.mozilla.org/r/82120/#review80752 > why is this moved? just hacking around and leftover? I don't know exactly why, but this is necessary to work on Mac. I know external-media-tests don't run on Mac, but left it here in name of consistency.
Comment on attachment 8796220 [details] Bug 1274980 part 2: Support Mac OS tests in tc intree config. modifying r? to match mozreview
Attachment #8796220 - Flags: review?(dustin) → review-
Comment on attachment 8796220 [details] Bug 1274980 part 2: Support Mac OS tests in tc intree config. https://reviewboard.mozilla.org/r/82120/#review81818 I see most of the things joel and I commented on fixed here. This is looking good! ::: taskcluster/taskgraph/transforms/task.py:437 (Diff revision 4) > + worker = task['worker'] > + artifacts = map(lambda artifact: { > + 'name': artifact['name'], > + 'path': artifact['path'], > + 'type': artifact['type'], > + 'expires': task_def['expires'], Ah, I misunderstood this in my last review -- it's setting `artifact[i].expires` explicitly to the task's expiration time. If tc-worker applies that default itself, then you could omit it here, otherwise this is fine. However, you still have `artifact['expires']` listed in the schema for the macosx-engine worker, but you end up ignoring that value. So either remove it from the schema, or add the logic to handle it here. ::: taskcluster/taskgraph/transforms/tests/make_task_description.py:256 (Diff revision 4) > + test_packages_url = ARTIFACT_URL.format('<build>', > + 'public/build/target.test_packages.json') > + mozharness_url = ARTIFACT_URL.format('<build>', > + 'public/build/mozharness.zip') > + > + taskdesc['worker-type'] = 'gecko-t-tc-worker/gecko-test-macosx64' In my last review, I was commenting on the workerType, rather than the provisioner -- we're standardizing on workerTypes named `gecko-t-macosx64` or the like. In this case, you might want to be more specific so that we have room to change things later: `gecko-t-osx-107` or something like that?
Attachment #8796220 - Flags: review?(dustin) → review-
Blocks: 1311814
Comment on attachment 8796220 [details] Bug 1274980 part 2: Support Mac OS tests in tc intree config. https://reviewboard.mozilla.org/r/82120/#review89798 Let's get the artifact-expiration thing sorted out. Other than that (and the mysterious `/` and some missing `x`'s), this looks good. ::: taskcluster/ci/desktop-test/tests.yml:20 (Diff revision 5) > no-read-buildbot-config: true > config: > by-test-platform: > win.*: > - unittests/win_taskcluster_unittest.py > + macosx.*/: what's going on with the trailing slash here? ::: taskcluster/ci/desktop-test/tests.yml:542 (Diff revision 5) > > reftest: > description: "Reftest run" > suite: reftest/reftest > treeherder-symbol: tc-R(R) > chunks: 8 remove this line to avoid a duplicate yaml key (I thought yaml barfed on duplicate keys....) ::: taskcluster/taskgraph/transforms/task.py:462 (Diff revision 5) > + artifacts = map(lambda artifact: { > + 'name': artifact['name'], > + 'path': artifact['path'], > + 'type': artifact['type'], > + 'expires': task_def['expires'], > + }, worker['artifacts']) This ignores `task['worker']['artifact'][i]['expires']`. If tc-worker applies that default itself, then you could omit it here, otherwise this is fine. However, you still have `artifact['expires']` listed in the schema for the macosx-engine worker, but you end up ignoring that value. So either remove it from the schema, or add the logic to handle it here. ::: taskcluster/taskgraph/transforms/tests/all_kinds.py:29 (Diff revision 5) > def set_worker_implementation(config, tests): > """Set the worker implementation based on the test platform.""" > for test in tests: > if test['test-platform'].startswith('win'): > test['worker-implementation'] = 'generic-worker' > + elif test['test-platform'].startswith('macos'): We should probably use `macosx` here since that's what the `by-test-platform` checks are looking for. Also, they are all looking for a trailing `/` in the test platform, so perhaps this should, too? ::: taskcluster/taskgraph/transforms/tests/desktop_test.py:25 (Diff revision 5) > > @transforms.add > def set_defaults(config, tests): > for test in tests: > - test['mozharness']['build-artifact-name'] = 'public/build/target.tar.bz2' > + build_platform = test['build-platform'] > + if build_platform.startswith('macos'): same here -- add `x`
Attachment #8796220 - Flags: review?(dustin) → review-
Comment on attachment 8796220 [details] Bug 1274980 part 2: Support Mac OS tests in tc intree config. https://reviewboard.mozilla.org/r/82120/#review90160 \o/
Attachment #8796220 - Flags: review?(dustin) → review+
Pushed by wcosta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b8760b0b11ac part 1: Add support for test platform regex match. r=dustin https://hg.mozilla.org/integration/autoland/rev/6228dfcfac3b part 2: Support Mac OS tests in tc intree config. r=dustin
Attachment #8808144 - Flags: review?(aryx.bugmail)
Attachment #8808144 - Flags: review?(aryx.bugmail) → review+
Backout by archaeopteryx@coole-files.de: https://hg.mozilla.org/integration/autoland/rev/9ee234b75a3e Backed out changeset c45f2ebe0e66 https://hg.mozilla.org/integration/autoland/rev/b95da309b398 Backed out changeset 6228dfcfac3b https://hg.mozilla.org/integration/autoland/rev/0a1dbd931f71 Backed out changeset b8760b0b11ac on suspicion of breaking marionette tests in taskcluster. r=backout on a CLOSED TREE
Backed out on suspicion of breaking marionette tests in taskcluster: https://hg.mozilla.org/integration/autoland/rev/0a1dbd931f7168740c6c5b97fc3e4c1cc0e824b2 https://hg.mozilla.org/integration/autoland/rev/b95da309b3985a8cf2efdac88fe974f8e34f0e26 https://hg.mozilla.org/integration/autoland/rev/9ee234b75a3ee9ec75bd6209e3bafe6b41bff1e2 First revision which ran Marionette on Taskcluster after the push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=dfce89359683ffb10419867556cc4fbf54218780 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=6229207&repo=autoland [task 2016-11-07T13:23:05.244125Z] 13:23:05 INFO - Running pre-action listener: _configure_marionette_virtualenv [task 2016-11-07T13:23:05.244770Z] 13:23:05 INFO - Running pre-action listener: _resource_record_pre_action [task 2016-11-07T13:23:05.244845Z] 13:23:05 INFO - Running main action method: create_virtualenv [task 2016-11-07T13:23:05.245525Z] 13:23:05 INFO - Creating virtualenv /home/worker/workspace/build/venv [task 2016-11-07T13:23:05.246146Z] 13:23:05 FATAL - The executable '/tools/buildbot/bin/python' is not found; not creating virtualenv! [task 2016-11-07T13:23:05.246242Z] 13:23:05 FATAL - Running post_fatal callback... [task 2016-11-07T13:23:05.246291Z] 13:23:05 FATAL - Exiting -1 [task 2016-11-07T13:23:05.246916Z] 13:23:05 INFO - Running post-action listener: _resource_record_post_action [task 2016-11-07T13:23:05.246993Z] 13:23:05 INFO - Running post-action listener: _start_resource_monitoring [task 2016-11-07T13:23:05.248243Z] 13:23:05 ERROR - Exception during post-action for create-virtualenv: Traceback (most recent call last): [task 2016-11-07T13:23:05.248339Z] 13:23:05 ERROR - File "/home/worker/workspace/mozharness/mozharness/base/script.py", line 1986, in run_action [task 2016-11-07T13:23:05.249010Z] 13:23:05 ERROR - method(action, success=success and self.return_code == 0) [task 2016-11-07T13:23:05.249755Z] 13:23:05 ERROR - File "/home/worker/workspace/mozharness/mozharness/base/python.py", line 572, in _start_resource_monitoring [task 2016-11-07T13:23:05.250429Z] 13:23:05 ERROR - self.activate_virtualenv() [task 2016-11-07T13:23:05.250526Z] 13:23:05 ERROR - File "/home/worker/workspace/mozharness/mozharness/base/python.py", line 495, in activate_virtualenv [task 2016-11-07T13:23:05.251231Z] 13:23:05 ERROR - execfile(activate, dict(__file__=activate)) [task 2016-11-07T13:23:05.251323Z] 13:23:05 ERROR - IOError: [Errno 2] No such file or directory: '/home/worker/workspace/build/venv/bin/activate_this.py' [task 2016-11-07T13:23:05.252008Z] 13:23:05 INFO - [mozharness: 2016-11-07 13:23:05.249349Z] Finished create-virtualenv step (failed) [task 2016-11-07T13:23:05.253425Z] 13:23:05 FATAL - Aborting due to failure in post-action listener. [task 2016-11-07T13:23:05.254044Z] 13:23:05 FATAL - Running post_fatal callback...
Flags: needinfo?(wcosta)
Pushed by wcosta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ef393dc07cb8 part 1: Add support for test platform regex match. r=dustin https://hg.mozilla.org/integration/autoland/rev/9d6b5736f9c8 part 2: Support Mac OS tests in tc intree config. r=dustin
Hidden on all trees until they get a little better about throwing cache permission denied errors and failing to upload a log, just click the "Excluded Jobs" in the gray bar at the top of the treeherder page to see them.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Flags: needinfo?(wcosta)
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: