1415965 - Move l10n-check to a separate test task

Status: NEW

(Firefox Build System :: General, enhancement)

Description

[Michael Shal [:mshal]]

I believe l10n-check is just used to verify the l10n machinery isn't broken in automation. Since we have the ability to trigger l10n jobs in Taskcluster, I believe the use-case for running l10n-check in every build is no longer valid. We should remove l10n-check to save time during the build task and invoke test tasks earlier.

Pike, Callek - does that sound right?

Comment 1

[Axel Hecht [:Pike]]

I disagree, the l10n check should test that the things in the l10n repacks are actually the things we want in the l10n repacks.

Things like checking that the manifest has the right entries etc should be done in l10n-check.

Basically, things where we know particular artifacts about localizations, and that they're showing up in localized builds in the expected way.

Comment 2

[Michael Shal [:mshal]]

(In reply to Axel Hecht [:Pike] from comment #1)
> I disagree, the l10n check should test that the things in the l10n repacks
> are actually the things we want in the l10n repacks.

Are you talking about this step in the locales Makefile where we verify that 'x-test' actually got written to update.locale?

https://dxr.mozilla.org/mozilla-central/rev/2535bad09d720e71a982f3f70dd6925f66ab8ec7/browser/locales/Makefile.in#217

If so, can we just check that update.locale was written correctly during the actual repack tasks and achieve the same level of verification without doing a separate repack during the build task?

Comment 3

[Axel Hecht [:Pike]]

As I said, "should". We had better test coverage at some point, https://hg.mozilla.org/mozilla-central/file/ec6c7dd9e25f/browser/locales/Makefile.in#l192, but we had landings backed out which that test depended on. Bug 1416000 is filed to land those tests again.

Comment 4

[Justin Wood (:Callek)]

:mshal,

So my current thinking is l10n-check should die, however until we have a replacement solution for "on-change l10n testing" its the only thing giving us a warning of build system changes that break l10n at the moment. (Noted, there is still many other ways for l10n to break that l10n-check doesn't catch, so if this is blocking work other than generic cleanup, I'm less worried about blocking).

I have plans/ideals to get a on-change l10n job enacted within the next quarter or so, and once that work is able to be live, I think we won't need l10n-check for that on-change guarantee.

Offhand though, what % of time are we talking per platform for l10n-check. As in will we save much by removing it ahead of the on-change work I am planning on.

Comment 5

[Gregory Szorc [:gps]]

In general, we are trying to move non-essential things out of the build tasks because the build task needs to finish before tests start executing. That delay annoys developers and slows down sheriffing.

There is little doubt in my mind that l10n-check needs to move out of the build task. Where exactly it goes, I'm not sure. But it appears it is adding 90-120s for Linux tasks, which is a non-trivial percentage of the overall task execution time. We cannot have l10n-check continue executing in the critical path of build tasks. If it needs to run as part of localized builds/tasks, fine. But it needs to be out of the primary build tasks.

Comment 6

[Axel Hecht [:Pike]]

They're a test task, and not part of localized builds. I wouldn't mind at all having them be a dedicated test task, that would make their asset handling a lot less fragile.

Note, bug 1385227 is up for review again, and reduces wall-clock time of the test as is significantly.

Comment 7

[(not currently active) Ted Mielczarek]

We'd like to get l10n-check out of the critical path of the build task. We should run it as a separate test task after the build. We might be able to reuse the l10n repack task definition, just pointing it at the x-test locale.