Closed Bug 646833 Opened 10 years ago Closed 8 years ago
l10n verify should take l10n-changesets into account
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.
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>> ?
Assignee: nobody → astoica
Attachment #532005 - Flags: review?(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
Attachment #532005 - Flags: review?(catlee) → review+
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)
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?
10 years ago
Assignee: astoica → catlee
Assignee: catlee → nobody
9 years ago
No longer blocks: 627271
9 years ago
Mass move of bugs to Release Automation component.
Component: Release Engineering → Release Engineering: Automation (Release Automation)
l10n verify is dead.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.