646833 - l10n verify should take l10n-changesets into account

Status: RESOLVED WONTFIX

(Release Engineering :: Release Automation: Other, defect, P4)

Description

[Chris AtLee [:catlee]]

l10n verify reports on ALL changes, regardless of if they're expected or not.

I think we should instead make it report success for a locale depending on if there were changes made to the locale according to the l10n changesets for the two releases being compared.

Comment 1

[Anamaria Stoica [:anamarias]]

Attachment: First draft

This is just a first draft, far from being complete. However, some input on the direction it is going would be most welcome.

So the idea is:
In the output of the diff command, I look for lines like (only locale files):
re.finditer('^diff -r <<(locale)_file_regex>>.*$', output, re.M) # | 'Only in ...' ??

If the locales found in the diff are exactly the same as the set of locales with different revisions in the old/new l10n_changesets, returns SUCCESS.
(if the diff contains diffs on other files than locales, it will always return SUCCESS, is this correct?)

Otherwise, returns FAILURE.

Should I be also looking for lines like  '^Only in ...' ??, right?
What is format of the locales files found in the diff <<(locale)_file_regex>> ?

Comment 2

[Chris AtLee [:catlee]]

Comment on attachment 532005 [details] [diff] [review]
First draft

This is a pretty good approach, but there are a few important details that need addressing:

- diffs_l10n_changesets needs to use asynchronous calls to fetch the data (look for other getPage() calls in buildbotcustom)

- diffs_l10n_changesets can't be called in __init__ and should instead run after the step has started. maybe call it in evaluateCommand? although, I'm not sure if you can return a deferred from there

Sample diff output can be found here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/3.6.17-candidates/build3/logs/release-mozilla-1.9.2-linux_l10n_verification-build6.txt.gz

Comment 3

[Anamaria Stoica [:anamarias]]

Attachment: Progress Buildbotcustom 1

It works like this, the L10nVerifyMetaDiff step was replaced by 2 new ones:
# diff -r new/diffs old/diffs > metadiff.out 
# python compare to changesets script metadiff.out l10nChUrl oldL10nChUrl
    # which, downloads new l10nchangesets from hg
    # downloads old l10nchangesets from hg
    # reads diffs file and makes checks

This way the steps (scripts) are run on the slave, and not the master.

A few questions:
1/ L10nVerifyFactory new init parameters: baseTag=None, oldBaseTag=None
If they are not provided, the 'verify_l10n_metadiff' step will fail. Should it ignore 
checking changesets files if not provided to ensure backwards compatibility and 
not be required to add in the 2 new parameters everywhere where L10nVerifyFactory is initialized?

Where should I fill in the config parameters? (baseTag, oldBaseTag)

2/ Changesets URL
changesetsRepoPath = 'build/buildbot-configs/file/%(tag)s/mozilla/l10n-changesets_mozilla-2.0'
Should the 2 last segments be customizable too? Which are the configs for them?

3/ Python step environment? When calling 'verify_l10n_metadiff' step, should I provide some 
kind of Python environment? I see some other steps use env=self.env.copy(), as args.

(see also patch for tools, containing the script)

Comment 4

[Anamaria Stoica [:anamarias]]

Attachment: Progress Tools 1

Script which downloads current and old l10n changesets

Parses 'metaDiffOutput' meta diffs file looking for diffs in locales.
    It returns true, if all diffs in 'metaDiffOutput' correspond to locales 
    with different revision numbers in ch1 and ch2, or present only in one 
    of them. Otherwise, returns false.

I tested this script with several scenarios and works great. Also tested it on the 120MB diff file that I got from you.

Haven't figured out how to test the factory in the previous patch. How should I test it?

Comment 5

[[e-mail] (:bhearsum)]

Mass move of bugs to Release Automation component.

Comment 6

[[e-mail] (:bhearsum)]

l10n verify is dead.