985418 - prioritize l10n nightlies below everything, regardless of branch

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Comment 1

[Justin Wood (:Callek)]

The trick here is I think it shouldn't be below our "play with me" branches, e.g. try, cedar, ash.

Otherwise we run the risk of never completing l10n in the time it takes us to trigger next set of nightlies, and lose lots of testing on our localized jobs.

Comment 2

[Massimo Gervasini [:massimo]]

Attachment: [buildbot-configs] updated priorityBuilder and added tests.patch

This patch introduces some changes in the builder priority sorting function and a test suite to check the priority of different builders/requests. As they are now, the patch passes all the tests but I am not sure I have touched all the possible builders/priorities scenarios

Comment 3

[Massimo Gervasini [:massimo]]

Attachment: [buildbot-configs] updated priorityBuilder and added tests2.patch

Added extra tests for l10n builders

Comment 4

[Massimo Gervasini [:massimo]]

Attachment: [buildbot-configs] updated priorityBuilder and added tests3.patch

minor tests refactoring; fixed missing () around multiple exceptions in builderPriority(...)

Comment 5

[Massimo Gervasini [:massimo]]

Attachment: [buildbot-configs] updated priorityBuilder and added tests4.patch

* builderPriority() returns a 3 element tuple: release, build_request_branch and submitted_at. (build_request_branch is: branch priority * 5 + request priority + builder priority)
* Updated tests
* run_test_suite.sh is still there because test-master.sh requires "/dev/shm"

Comment 6

[Chris AtLee [:catlee]]

Comment on attachment 8397771 [details] [diff] [review]
[buildbot-configs] updated priorityBuilder and added tests4.patch

Review of attachment 8397771 [details] [diff] [review]:
-----------------------------------------------------------------

::: mozilla/master_common.py
@@ +71,5 @@
>  def builderPriority(builder, request):
>      """
>      Our builder sorting function
> +    Returns (release, magic_number, submitted_at)
> +    where magic_number = 5 * branch_priority + req_priority + builder_priority

maybe want to clarify here that req_priority is actually the negative of the priority in the DB?

@@ +101,3 @@
>  
>      # Default builder priority is 100
> +    builder_priority = 1

comment needs updating. let's change the default to 0 while we're at it.

::: mozilla/test/test_builderPriority.py
@@ +220,5 @@
> +        priority_2 = l10n_builder.get_priority_request(high_priority_request)
> +        expected = [priority_0, priority_1, priority_2]
> +        import random
> +        # shuffle elements
> +        result = sorted(expected, key=lambda *args: random.random())

I don't think this is a good way to test that they get ordered properly. It allows for some indeterminism in the tests in the case we break the sorting function later.

Better to put the result in an order you don't expect, and then sort it, and make sure it matches your expected result. e.g.
result = sorted(reversed(expected))
self.assertTrue(result == expected)

Comment 7

[Massimo Gervasini [:massimo]]

Attachment: [buildbot-configs] updated priorityBuilder and added tests5.patch

Thanks for the feedback, catlee

this new patch should fix all the issues you have found.
This is still a feedback request because I have removed the run-test-suite.sh file (was added in a previous patch) and updated test-masters.sh so when executing:

test-masters.sh run-tests

it only runs the trial test function skipping all master tests part

Comment 8

[Chris AtLee [:catlee]]

Comment on attachment 8398467 [details] [diff] [review]
[buildbot-configs] updated priorityBuilder and added tests5.patch

Review of attachment 8398467 [details] [diff] [review]:
-----------------------------------------------------------------

::: test-masters.sh
@@ +41,5 @@
> +}
> +
> +if [ "$1" == "run-tests" ]
> +then
> +    # run trial and exist

exit!

Comment 9

[Massimo Gervasini [:massimo]]

Comment on attachment 8398467 [details] [diff] [review]
[buildbot-configs] updated priorityBuilder and added tests5.patch

got r+ from catlee. Landed

Comment 10

[John Hopkins (:jhopkins)]

in production.