NOTE: I've moved this to the public list, no need to talk in private about this.
On Tue, Feb 10, 2015 at 6:35 PM, Daniel Manrique <[email protected]> wrote: > On Tue, Feb 10, 2015 at 12:29 PM, Zygmunt Krynicki > <[email protected]> wrote: >> >> >> On Tue, Feb 10, 2015 at 4:51 PM, Daniel Manrique >> <[email protected]> wrote: >>> >>> >>> >>> On Tue, Feb 10, 2015 at 10:44 AM, Sylvain Pineau >>> <[email protected]> wrote: >>>> >>>> Hello >>>> >>>> To catch as early as possible job id modifications that could break test >>>> plans depending on our providers, >>>> I've creating a small bash script [1] that will run everyday at 00:30 on >>>> lauma in the certification contab. >>>> >>>> It will only send mails to the mailing list if a job id has been >>>> added/renamed/deleted the day before. >>>> I checked that using an arbitrary date far in the past it triggers the >>>> cron mail (see below) and that >>>> nothing happens when ... there's nothing to report :) >>> >>> >>> >>> It's a great idea :) >>> >>> I remember checkbox legacy had some tests that would fail package building >>> if certain checks in job files didn't pass, this gave a measure of >>> protection against submitting bad job files. With the current structure this >>> may be harder to do, but it may also be worth considering; if the tree >>> contains weird jobs, packages will simply not build. >>> >>> Examples, we ensured that all job files were declared in setup.cfg, added >>> to the potfiles catalog and added to the local.txt "master" local job file: >>> >>> def test_job_files_in_setup_cfg(self): >> >> >> What is this one doing? > > It checks that job files are declared in setup.cfg, this may not be > needed now but it was back then, since the debian packaging stuff was > responsible for collecting and installing job files. The consequence > could be a missing job file that was referenced in a whitelist, > causing problems. > >> >>> >>> def test_job_files_in_potfiles(self): >> >> >> We don't have a check like this. Now that we understand pot files and po >> files in plainbox, we could do that >> >>> >>> def test_job_files_in_local_txt(self): >> >> >> I suspect this one is totally obsolete now > > Yes, we don't use that anymore. > >> >>> >>> >>> Also, we validated all jobs with a few basic checks. Some of these are >>> done by plainbox (I recall because I worked a bit with Zygmunt on ensuring >>> job validation considered these cases) but I don't remember if this is done >>> at build time (plus as job definitions >> >> >> Yes, we validate during build >> >>> >>> have evolved, some of these checks are no longer even valid; in >>> particular, many new jobs will break jobs_comply_with_schema and we no >>> longer rely on parsable descriptions): >>> >>> def test_job_files_valid(self): >> >> >> I suspect this is something we already do > > This test is very simplistic, it just tries to load all the job files > and if it comes up with 0 jobs, then something is horribly broken :) > (but just 1 loaded job would make this test happy). > >> >>> >>> def test_all_messages_have_name(self): >> >> >> Messages? > > Remember in checkbox-legacy jobs were internally called "messages"? > that's why this method has this name. It just ensures all jobs have a > name: attribute. This was migrated to id and I'm pretty sure plainbox > would complain if a job had no id :) > >> >>> >>> def test_all_messages_have_command_or_description(self): >>> def test_shell_jobs_have_description(self): >>> def test_shell_jobs_with_root_have_needed_environ(self): >> >> >> We do those >> >>> >>> def test_jobs_comply_with_schema(self): >> >> >> Schema? > > Particularly when uploading to hexr, jobs that had custom fields would > crash the parser. This is no longer the case with plainbox, but back > then we needed to ensure jobs complied with the "schema", defined as > the list of known fields and their value types. This is clearly not > needed with plainbox since we use free-form, "extensible" jobs now. > >> >>> >>> def test_verify_interact_jobs_have_command(self): >> >> >> We do this >> >>> >>> def test_verify_manual_jobs_have_parsable_description(self): >> >> >> We explicitly don't try to do this as there are no hidden types of syntax in >> descriptions in general. > > +1 on that :) > >> >> >> If anyone wants to port any of those over to plainbox they belong in the >> unit validation chain. > > Cool! Some of them may still be relevant. My comment was geared > towards doing this at build (or test) time to protect against building > packages with borked data. But I'm glad to see we already do some of > this (see, I kinda remembered we did!). Thanks, I think we have all of those EXCEPT for the one that is related to i18n, which is pretty interesting. We could now write a simple validation method for FileType with unit.role == i18n and unit.path.endswith('POTFILES.in') def files_mentioned_by_POTFILES_in(pathname): files_seen = [] for line in open(pathname, 'rt', encoding='UTF-8'): if re.match("\[encoding: .+\]"): continue # assume utf-8 m = re.match("\[type: [^]]\s+(.+)") if not m: raise SyntaxError(...) files_seen.append(m.groups(1)) return frozenset(files_seen) def check_POTFILES_in(unit): assert unit.Meta.name == 'file' and unit.path.endswith("POTFILES.in") assert unit.provider.base_dir is not None files_mentioned = files_mentined_bt_POTFILES_in(unit) for unit in [unit for unit in unit.provider.unit_list if unit.Meta.name == 'file']: relative_path = os.path.relpath(unit.path, provider.base_dir) if relative_path not in files_mentioned: # self is the validator class self.warning("{} not mentioned by {}".format(relative_path, unit.path) The code above is hand-written and untested but the rough idea should shine through, any takers? Best regards ZK -- Mailing list: https://launchpad.net/~checkbox-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~checkbox-dev More help : https://help.launchpad.net/ListHelp
