Closed Bug 969988 Opened 10 years ago Closed 10 years ago

ASan is the only Linux platform running the separate jittest suite on try


(Release Engineering :: General, defect)

Not set


(Not tracked)



(Reporter: philor, Assigned: kmoir)




(4 files, 2 obsolete files)

I have a vague memory of a bug where we thought that we were turning off having jittest run on both the ubuntu and the fedora slaves, which I suspect is the reason why got no Linux jittests other than on ASan.
Blocks: 969991
So we want jittests to run on all linux platforms on try, not just ASan
Blocks: 973900
It looks like the problem is at [1] which I guess was added to prevent scheduling on both fedora and ubuntu slaves on Cedar. This condition should probably be moved to where we add the tests for Cedar so that it doesn't affect try.

Assignee: nobody → dminor
Attachment #8383685 - Flags: review?(bhearsum)
Comment on attachment 8383685 [details] [diff] [review]
Make logic to skip Fedora slaves only apply to Cedar

Review of attachment 8383685 [details] [diff] [review]:

our configs suck
Attachment #8383685 - Flags: review?(bhearsum) → review+
buildbot-configs patch is merged and live in production! :)
My patch did not work, these are still running on ASAN only.
I'm happy to redo this patch, but I could use some help identifying the root cause of why these are only being scheduled on ASAN for try. Scheduling for cedar is fine.
Flags: needinfo?(bhearsum)
(In reply to Dan Minor [:dminor] from comment #8)
> I'm happy to redo this patch, but I could use some help identifying the root
> cause of why these are only being scheduled on ASAN for try. Scheduling for
> cedar is fine.

Looks like the first check of the block is wrong:

>>> 1 in ([2,3] or [4,5])
>>> 1 in ([2,3] or [4,5,1])

You should probably check those separately, eg:
if platform not in cedar_platforms or platform not in try_platforms:
Flags: needinfo?(bhearsum)
D'oh, thanks for catching that. And here I thought it was something mysterious with buildbot.
Attached patch Fix initial platform check (obsolete) — Splinter Review
Switch not in (A or B) to not in A and not in B. If A is not an empty list, (A or B) returns A, so the original code never checked inclusion in B.
Attachment #8392842 - Flags: review?(bhearsum)
Comment on attachment 8392842 [details] [diff] [review]
Fix initial platform check

Review of attachment 8392842 [details] [diff] [review]:

::: mozilla-tests/
@@ +1822,5 @@
>                          pass
>  # Enable jittests on cedar
>  for platform in PLATFORMS.keys():
> +    if platform not in BRANCHES['cedar']['platforms'] and platform not in BRANCHES['try']['platforms']:

I think you want "or" not "and". Otherwise you can't control them for cedar and try independently. r=me if you fix that on landing.
Attachment #8392842 - Flags: review?(bhearsum) → review-
I split the code to handle cedar and try separately. I think this will fix the scheduling problem and be easier to read.
Attachment #8392842 - Attachment is obsolete: true
Attachment #8395635 - Flags: review?(bhearsum)
Comment on attachment 8395635 [details] [diff] [review]
Fix platform check

Review of attachment 8395635 [details] [diff] [review]:

Looks fine to me now
Attachment #8395635 - Flags: review?(bhearsum) → review+
in production
Looks like my new patch also does not schedule on anything other than ASAN for try:

kmoir, since you're looking at bug 973900, do you mind having a look at this bug as well? I don't seem to be getting anywhere with this.
Flags: needinfo?(kmoir)
Assignee: dminor → kmoir
Flags: needinfo?(kmoir)
I probably can get this sorted out in a week or so. I've setup a new test master environment, just looking at the configs to see what's happening.
Attached patch bug969988.patch (obsolete) — Splinter Review
So the problem is that the jittests added starting here

but then are disabled here

because certain tests are deleted unless they reside on cedar

looking more closely, it looks like a 3/17 merge day patch wasn't removed starting here

I've tested removing the merge day stuff on my test master and the proper jittests appear.  Asking rail for comment since he was on the last merge duty.
Attachment #8398040 - Flags: review?(rail)
Attached patch bug969988.patchSplinter Review
Talked to rail, the merge day stuff is still needed for bug 875164.  So modified the patch to just not delete the tests on try
Attachment #8398040 - Attachment is obsolete: true
Attachment #8398040 - Flags: review?(rail)
Attachment #8398104 - Flags: review?(rail)
Comment on attachment 8398104 [details] [diff] [review]

A nit:
Can you use the following instead:
if branch in ("cedar", "try"):
Attachment #8398104 - Flags: review?(rail) → review+
Attached patch updated patchSplinter Review
in production.
Comment on attachment 8398123 [details] [diff] [review]
updated patch

checked in before this morning's reconfig
Attachment #8398123 - Flags: checked-in+
Verified in tbpl and buildapi that tests are running/being scheduled
Closed: 10 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.