Merge lp:~oem-qa/checkbox/patch_selection_dialog_sooner_and_whitelist_ordering into lp:checkbox

[Javier Collado]

This is a replacement for the _find_matching_messages method in the JobStore class. A new method _update_cache has been written and some changes have been made to the add method.

The strategy is to maintain some sets to cache important values from persisted data (filename, job name and job suite). The files to read from the store are selected based on the cached values, so there is no need to read every file as in the previous implementation.

Assuming that this fix is correct, the performance improvement is around one order of magnitude in the machines tested.

[lp:~oem-qa/checkbox/patch_selection_dialog_sooner_and_whitelist_ordering]

799. By Marc Tardif on 2010-04-22

Added support for platform floppy devices.

800. By Marc Tardif on 2010-04-27

Removed PPA tags.

801. By Marc Tardif on 2010-04-27

Merged from trunk.

[Javier Collado (javier.collado)]

It seems that this patch ins't working completely fine after all. In fact, I think that it has some side effects with regard to the tree hierarchy in the selection dialog (LP: 571815). Any input would be appreciated.

[lp:~oem-qa/checkbox/patch_selection_dialog_sooner_and_whitelist_ordering]

802. By Marc Tardif on 2010-05-03

Fixed cpu_scaling_test to return zero if cpu frequency is not supported.

803. By Marc Tardif on 2010-05-04

Increased version number.

804. By Marc Tardif on 2010-05-04

Merged from trunk.

805. By Marc Tardif on 2010-05-14

Added support for comments in templates and extended the filter_templates script to make use of these comments.

806. By Marc Tardif on 2010-05-17

Merged from branch bug-397944.

807. By Marc Tardif on 2010-05-17

Fixed branch bug-397944 which introduced an invalid job definition.

808. By Marc Tardif on 2010-05-18

Updated po files.

809. By Marc Tardif on 2010-05-18

Moved icons directory from the data subdirectory to the root.

810. By Marc Tardif on 2010-05-18

Changed regular expression for data directory from *.txt to *.

811. By Marc Tardif on 2010-05-24

Updated changelog.

812. By Marc Tardif on 2010-05-24

Merged from checkbox-urwid branch.

813. By Marc Tardif on 2010-05-24

Merged checkbox audio branch.

814. By Marc Tardif on 2010-05-24

Merged from office_docs branch.

815. By Marc Tardif on 2010-06-05

Casting port to int when split using urllib.

816. By Marc Tardif on 2010-06-15

Added checkbox-urwid generated files to .bzrignore.

817. By Marc Tardif on 2010-06-15

Merged from cpu_scaling_exectime_fix branch.

818. By Marc Tardif on 2010-06-15

Merged from manual_nautilus_create_delete branch.

819. By Marc Tardif on 2010-06-15

Merged from internet_test_extensions branch.

820. By Marc Tardif on 2010-06-16

Merged from cts-iter_4-item_65 branch.

821. By Marc Tardif on 2010-06-18

Merged from tests-after-sleep branch.

822. By Marc Tardif on 2010-06-22

Merged from media-cards branch.

823. By Marc Tardif on 2010-06-22

Updated changelog.

824. By Marc Tardif on 2010-06-22

Merged checkbox_0.10.1-1~ppa0 [a=autoppa].

  * Compatibility PPA.

825. By Marc Tardif on 2010-06-22

Merged from listen_nm_dbus_after_sleep branch.

826. By Marc Tardif on 2010-06-23

Merged from tests-after-sleep branch.

827. By Marc Tardif on 2010-06-29

Merged from trunk.

828. By Marc Tardif on 2010-06-29

Merged from audio_v2 branch.

829. By Marc Tardif on 2010-06-29

Merged from reorganize-data branch.

830. By Marc Tardif on 2010-06-30

Updated sleep.txt.in to use shell instead of local for commands run before suspend.

831. By Marc Tardif on 2010-07-02

Merged from check_volume_after_suspend_v2 branch.

832. By Marc Tardif on 2010-07-02

Merged from firewire-hdd branch.

833. By Marc Tardif on 2010-07-05

Fixed typo in sleep jobs.

834. By Marc Tardif on 2010-07-07

Merged from media_add_CF test.

835. By Marc Tardif on 2010-07-12

Merged from cts-iter_6-item_74 branch.

836. By Marc Tardif on 2010-07-12

Merged from bluetooth_detect_after_suspend branch.

837. By Marc Tardif on 2010-07-15

Merged from trunk.

838. By Marc Tardif on 2010-07-15

Merged from add-firefox-tests branch.

839. By Marc Tardif on 2010-07-15

Merged from add-empathy-tests branch.

[Javier Collado (javier.collado)]

Hello,

A bug was identified that has been fixed and now it seems that all problems are gone. Please review it and let me know if it's ready to be integrated.

Best regards,
    Javier

[lp:~oem-qa/checkbox/patch_selection_dialog_sooner_and_whitelist_ordering]

840. By Marc Tardif on 2010-07-19

Reporting bugs against alsa-base and xorg instead of gst0.10-python which fixes bug #607214.

841. By Marc Tardif on 2010-07-19

Enabled apport dialog which diappeared which fixes bug #607217.

842. By Marc Tardif on 2010-07-22

Firing report-job when reporting remote message.

843. By Marc Tardif <cr3@lime> on 2010-07-22

Moved makedir /var/cache/checkbox to root environment.

844. By Marc Tardif on 2010-07-24

Fixed typo in potfile to generate i18n strings for user_apps.txt.in.

845. By Marc Tardif on 2010-07-29

Merged from checkbox-pretty-report branch.

846. By Marc Tardif on 2010-08-13

Reduced size of some data files.

847. By Marc Tardif on 2010-08-13

Renamed a few more data files for consistency purposes.

848. By Marc Tardif on 2010-08-13

Updated changelog.

849. By Marc Tardif on 2010-08-13

Updated changelog again.

850. By Marc Tardif on 2010-08-18

Merged from fix-network-test branch.

851. By Marc Tardif on 2010-08-19

Replaced borrowed SWF file with another generated from source.

852. By Marc Tardif on 2010-08-19

Updated po files.

853. By Marc Tardif on 2010-09-14

Added verification of SSL validity which fixes bug #625076.

854. By Marc Tardif on 2010-09-14

Improved audio test questions.

855. By Marc Tardif on 2010-09-14

Added changelog entry for candidate revision.

856. By Marc Tardif on 2010-09-15

Updated pot file.

857. By Marc Tardif on 2010-09-15

Changed connection request to use the path rather than the full url.

858. By Marc Tardif on 2010-09-17

Increased version number

859. By Marc Tardif on 2010-11-25

Merged from cli_no_answer branch.

860. By Marc Tardif on 2010-11-29

Changed connection request back to use the full url instead rather than the path.

861. By Marc Tardif on 2010-12-08

Replaced external plugin with remote plugin in autotest and ltp scripts.

862. By Marc Tardif on 2010-12-10

Merged from testsprint-checkbox-base-sru-changes.

863. By Marc Tardif on 2010-12-23

Updated parsing of config parameters which fixes bug #689140.

864. By Marc Tardif on 2011-01-12

Merged from trunk.

865. By Marc Tardif on 2011-01-12

Merged from whitelist_file_comments_support.

866. By Marc Tardif on 2011-01-12

Fixed jobs_info plugin to strip commented lines in whitelist and blacklist files which might start with a space.

867. By Marc Tardif on 2011-01-12

Added additional logging to the reactor when firing messages.

868. By Marc Tardif on 2011-01-26

Merged from checkbox-add-pm-utils-requirement branch.

869. By Marc Tardif on 2011-01-27

Added support for TOUCH devices.

870. By Jeff Lane  on 2011-02-02

Merged cr3s fix for bug #561816

871. By Marc Tardif on 2011-02-02

Merged from bladernr to add _attachment suffix and lsmod attachment.

872. By Marc Tardif on 2011-02-02

Updated pot file.

873. By Marc Tardif on 2011-02-02

Fixed memory persistence to be rooted like file persistence.

874. By Marc Tardif on 2011-02-02

Fixed persist module to support not being given a filename.

875. By Marc Tardif on 2011-02-02

Extended persist_info plugin to answer to both begin and prompt-begin messages.

876. By Marc Tardif on 2011-02-03

Added support for Python 2.5 in checkbox.lib.transport.

877. By Marc Tardif on 2011-02-08

Merged from audio_test_failing branch.

878. By Marc Tardif on 2011-02-09

Merged from syslog branch.

879. By Marc Tardif on 2011-02-11

Added stop signal when executing messages.

880. By Marc Tardif on 2011-02-14

Migrated UI from libglade to gtkbuilder which fixes bug #403534.

881. By Marc Tardif on 2011-02-14

Defining default options in checkbox.application rather than CHECKBOX_OPTIONS environment variable.

882. By Marc Tardif on 2011-02-14

Added changelog entry for candidate revision.

883. By Marc Tardif on 2011-02-15

Merged from 719552.

884. By Marc Tardif on 2011-02-16

Changed description of nautilus_file_copy job which fixes bug #709688.

885. By Marc Tardif on 2011-02-18

Fixed title in progress dialog.

886. By Marc Tardif on 2011-02-25

Updated changelog with new upstream release.

887. By Marc Tardif on 2011-03-01

Updated pot file.

888. By Marc Tardif on 2011-03-01

Merged from 727411.

889. By Marc Tardif on 2011-03-11

Merged from 691241.

890. By Marc Tardif on 2011-03-11

Added support for trying to submit twice which fixes bug #531010.

891. By Marc Tardif on 2011-03-11

Merged from 642001.

892. By Jeff Lane  on 2011-03-11

Merged cli-cleanup for bug #221400

893. By Marc Tardif on 2011-03-14

Merged from checkbox-bug-fixes.

894. By Jeff Lane  on 2011-03-17

Land translation work by Mahyuddin Susanto via Michael Terry

895. By Jeff Lane  on 2011-03-17

Merged cr3 changes to changelog and control

896. By Marc Tardif on 2011-03-18

Added changelog entry for candidate revision.

897. By Jeff Lane  on 2011-03-29

Commited Gerhard Burgers punctuation fix for LP #744167

898. By Marc Tardif on 2011-03-29

Merged from 553777.

899. By Marc Tardif on 2011-03-29

Moved Gerhard Burger in changelog from 0.11.2 to 0.11.3.

900. By Marc Tardif on 2011-03-30

Fixed missing capital letter in sleep_state_test description.

901. By Marc Tardif on 2011-03-30

Merged from 744964.

902. By Jeff Lane  on 2011-04-05

Merged Marc Tardif fix for lp:729431

903. By Jeff Lane  on 2011-04-05

Merged Marc Tardif fixes to hibernate test for lp:630785

904. By Jeff Lane  on 2011-04-05

Merged addition of rtc test to sleep.txt.in to meet dependencies

905. By Marc Tardif on 2011-04-05

Reintroduced pm-utils requirements for jobs calling the sleep_test command and update po files.

906. By Marc Tardif on 2011-04-05

Merged checkbox log to apport report.

907. By Marc Tardif on 2011-04-05

Added changelog entry for candidate revision.

908. By Marc Tardif on 2011-04-18

Merged branch to enable camera detect test.

909. By Jeff Lane  on 2011-04-22

Merged Carl Milettes fix for lp:507943 (disk_bench_test hard coded drive)

910. By Marc Tardif on 2011-05-05

Fixed eval of resources with names like list item names.

911. By Marc Tardif on 2011-05-17

Merged from pygi-gtk3-port.

912. By Marc Tardif on 2011-05-17

Removed dead pixel test.

913. By Marc Tardif on 2011-05-19

Merged from 773667.

914. By Marc Tardif on 2011-05-24

Merged from 776734.

915. By Marc Tardif on 2011-05-25

Merged from 786924.

916. By Marc Tardif on 2011-05-25

Merged from 776712.

917. By Jeff Lane  on 2011-05-31

Applied Marc Tardifs changes to allow for remote submission (send results from a system other than the system under test)

918. By Marc Tardif on 2011-06-03

Merged from 588539.

919. By Marc Tardif on 2011-06-06

Merged from 665299.

920. By Marc Tardif on 2011-06-07

Wrapped submission stream to check for illegal xml characters.

921. By Marc Tardif on 2011-06-08

Replaced dependency on pytz with dateutil.

922. By Marc Tardif on 2011-06-10

Merged from 621880.

923. By Marc Tardif on 2011-06-13

Merged from add_camera_tests.

924. By Marc Tardif on 2011-06-13

Merged from fix_get_pointer_error.

925. By Marc Tardif on 2011-06-13

Incremented version.

926. By Marc Tardif on 2011-06-16

Merged from 798200.

927. By Marc Tardif on 2011-06-16

Merged from patch_resourceobject_contains.

928. By Marc Tardif on 2011-06-16

Merged from checkbox-fix-pygi-misbehaviors.

929. By Marc Tardif on 2011-06-16

Merged from 744190.

930. By Marc Tardif on 2011-06-16

Merged from add_interface_option_for_multi_nic_test.

931. By Marc Tardif on 2011-06-21

Merged from test-name-in-report-a-bug.

932. By Marc Tardif on 2011-06-21

Incremented version.

933. By Marc Tardif on 2011-06-28

Only reading CHECKBOX_* environment variables in config which fixes bug #802458.

934. By Marc Tardif on 2011-06-29

Added preliminary default.whitelist.

935. By Marc Tardif on 2011-06-29

Imported scripts and jobs from Platform Services.

936. By Daniel Manrique on 2011-06-29

merge patch_log_format_object

937. By Marc Tardif on 2011-06-29

Imported plugins from Platform Services.

938. By Daniel Manrique on 2011-06-29

merged patch_apport_interface_yesno

939. By Marc Tardif on 2011-06-29

Merged switch to dh_python2 and debhelper7 which fixes bug #788514.

940. By Brendan Donegan on 2011-06-30

Removed some tests that shouldn't have appeared in the whitelist.

941. By Marc Tardif on 2011-07-01

Merged from improve_failed_test_message.

942. By Daniel Manrique on 2011-07-01

merged Barry Warsaw's fix for problem with non-existing executables while building from clean tree

943. By Marc Tardif on 2011-07-06

Merged from camera_test_grouping.

944. By Marc Tardif on 2011-07-07

Merged from story325_test_organisation.

945. By Marc Tardif on 2011-07-07

Updated pot file.

946. By Daniel Manrique on 2011-07-08

merged missing_suites branch

[Daniel Manrique (roadmr)]

OK, so this merge request includes two new features. The first is the caching mechanism to avoid having to open and process all the message files in $CHECKBOX_DATA/store/0/ each time a new job is added. This is supposed to provide a significant speed boost. For my testing (~100 tests) I didn't see any measurable improvements in speed but it may just be that I have too few tests.

The second is, of course, the "whitelist ordering", which attempts to execute jobs in the order specified in the whitelist.

I found two issues while testing this code against latest checkbox trunk (r946).

The first is that when a job has no suite, its job file gets deleted, and since the JobStore cache doesn't register this, it still thinks the file is present. When doing the whitelist reordering the absence of some files that are expected to be there throw an exception (checkbox/jobs.py, line 138), and the remaining files don't get ordered. Granted, jobs without suites cause other problems so it's ideal to have all jobs within suites, but maybe we could make this a bit more robust? the ideal way would be to keep the cache in sync with what's in the directory, and catching possible exceptions (in my test I just skipped nonexistent files and it seemed to work well) would be a nice safety net.

Second issue is that I don't see dependencies being ordered correctly. Say I have in a whitelist:

input/keyboard
input/mouse

if input/keyboard has depends: input/mouse, I'd expect input/mouse to be identified as a dependency and be run before keyboard. In practice this doesn't happen and checkbox presents the keyboard test first. This means that dependencies have to be expressed in two places: whitelist and in the actual job definitions. I wonder if this could be made so that dependencies override the order given in the whitelist, as we absolutely need them to run in the order determined by the topological sort. Or maybe I'm doing it wrong? :)

Javier and Sylvain, I'd really appreciate your input on these two issues so we can move this forward and hopefully have these changes merged soon, as they are useful features that will make testing easier for all.

Thanks and apologies, both for the delay to start reviewing the code and the time it took for me to understand and test it.

review: Needs Information

[lp:~oem-qa/checkbox/patch_selection_dialog_sooner_and_whitelist_ordering]

947. By Daniel Manrique on 2011-07-08

Fixed typo in local job definition which caused exception and test to not be run

948. By Daniel Manrique on 2011-07-11

merge story221_move_30_cycles_to_stress

949. By Daniel Manrique on 2011-07-11

merged fixup_powermanagement_suspends

950. By Daniel Manrique on 2011-07-11

merged story221_cpu_before_after_suspend

[Javier Collado (javier.collado)]

@Daniel

> OK, so this merge request includes two new features.

Yes, that's correct the second feature was developed in a separate branch, but since it depends on the first one, it was joined together in the merge request.

> provide a significant speed boost. For my testing (~100 tests) I didn't see
> any measurable improvements in speed but it may just be that I have too few
> tests.

I don't really have any numbers right now, but when the number of test cases grows, it takes minutes to get the selection dialog rathen than seconds.

> The first is that when a job has no suite, its job file gets deleted, and
> since the JobStore cache doesn't register this, it still thinks the file is
> present.

That's interesting, I wasn't aware of this problem. For sure this has to be fixed. Anyway, as far as I know, the certification website doesn't support the submission of results for test cases that aren't included in any test suite so there isn't a use case for this at the moment.

> could make this a bit more robust? the ideal way would be to keep the cache in
> sync with what's in the directory, and catching possible exceptions (in my
> test I just skipped nonexistent files and it seemed to work well) would be a
> nice safety net.

I agree.

> Second issue is that I don't see dependencies being ordered correctly.

Dependency checking was left out of the ordering feature to keep it as simple as possible so the user it's expected to take this into account. The way we handle this is through checkbox-editor, which is the tool we use not only to edit test cases, but also to generate whitelist files. This tool make sure that suite appear before cases and that dependencies are honored (there's still a problem if the user accidentally creates a circular dependency, though).

> Thanks and apologies, both for the delay to start reviewing the code and the
> time it took for me to understand and test it.

No apologies needed, you were not even part of the team when the merge request was created. Thanks for your efforts. Despite the merge request works for us, I don't think the code is ready to be merged so your comments are appreciated.

[Daniel Manrique (roadmr)]

> @Daniel
>
> > OK, so this merge request includes two new features.
>
> Yes, that's correct the second feature was developed in a separate branch, but
> since it depends on the first one, it was joined together in the merge
> request.

No problem. Sylvain has agreed to kill the whitelist_ordering branch and focus on this one which contains the same code.

> > The first is that when a job has no suite, its job file gets deleted, and
> > since the JobStore cache doesn't register this, it still thinks the file is
> > present.
>
> That's interesting, I wasn't aware of this problem. For sure this has to be
> fixed. Anyway, as far as I know, the certification website doesn't support the
> submission of results for test cases that aren't included in any test suite so
> there isn't a use case for this at the moment.

I think suiteless jobs are supported in checkbox proper, and since this feature is to be integrated there, rather than in checkbox-certification or checkbox-oem, it makes sense to handle these jobs more gracefully, even if keeping the cache in sync is a bit more work. The "just ignore them" strategy works if we assume that no job will be suitless, but that might baffle people who are not familiar with that unwritten convention.

> > Second issue is that I don't see dependencies being ordered correctly.
>
> Dependency checking was left out of the ordering feature to keep it as simple
> as possible so the user it's expected to take this into account. The way we
> handle this is through checkbox-editor, which is the tool we use not only to
> edit test cases, but also to generate whitelist files. This tool make sure
> that suite appear before cases and that dependencies are honored (there's
> still a problem if the user accidentally creates a circular dependency,
> though).

This might be confusing for people creating whitelists manually (us!). I'll try to think of some way to make this more intuitive, but as you mention, keeping things simple would be good.

Thanks for your input, hopefully we'll be able to come up with some way to improve these features.

[Javier Collado (javier.collado)]

@Daniel

Before going forward with more changes in this branch, I'd like to know if you agree on the design of the cache. If you have an idea about how to approach this in a different way, I'd like to hear that.

> I think suiteless jobs are supported in checkbox proper, and since this
> feature is to be integrated there, rather than in checkbox-certification or
> checkbox-oem, it makes sense to handle these jobs more gracefully, even if
> keeping the cache in sync is a bit more work. The "just ignore them" strategy
> works if we assume that no job will be suitless, but that might baffle people
> who are not familiar with that unwritten convention.

While I still agree it's a bug, there's not great benefit in implementing this. We even have a patch to warn the user when a test case not in any test suite is detected:
lp:~oem-qa/checkbox/patch_display_error_on_whitelist_pattern_not_matched

The patch also warns the user when some of the patterns in the whitelist didn't match any job names. In case you would like to consider some of these changes in a separate merge proposal, please let me know and I'll create it.

> This might be confusing for people creating whitelists manually (us!). I'll
> try to think of some way to make this more intuitive, but as you mention,
> keeping things simple would be good.

As I said, I'm open to any suggestions. Thanks.

[lp:~oem-qa/checkbox/patch_selection_dialog_sooner_and_whitelist_ordering]

951. By Daniel Manrique on 2011-07-13

merge patch_exit_on_whitelist_file_not_found

952. By Daniel Manrique on 2011-07-13

merged patch_display_text_in_show_tree_method

953. By Daniel Manrique on 2011-07-13

merged from story221_memory_suspend

954. By Marc Tardif on 2011-07-15

Merged from fix-backend-protocol.

955. By Marc Tardif on 2011-07-15

Merged from 804369-808423.

956. By Daniel Manrique on 2011-07-15

Change mention of OpenOffice to LibreOffice (thanks to David Baucum)

[Daniel Manrique (roadmr)]

> @Daniel
>
> Before going forward with more changes in this branch, I'd like to know if you
> agree on the design of the cache. If you have an idea about how to approach
> this in a different way, I'd like to hear that.

I see you're using a set and two dictionaries for the cache. A bit further down the code there's some juggling of these three data structures, which have to be kept in sync at various places in the code, and at some other places there are some "staircase conditionals" that might be better off if abstracted so the JobStore deals with an abstraction instead of with these three structures. Do you think the code could be made clearer, as well as more "encapsulated", by abstracting all this handling in a JobCache class? It's just an idea and I'm not sure if it would make the code clearer.

> > I think suiteless jobs are supported in checkbox proper, and since this
> > feature is to be integrated there, rather than in checkbox-certification or
> > checkbox-oem, it makes sense to handle these jobs more gracefully, even if
> > keeping the cache in sync is a bit more work. The "just ignore them"
> strategy
> > works if we assume that no job will be suitless, but that might baffle
> people
> > who are not familiar with that unwritten convention.
>
> While I still agree it's a bug, there's not great benefit in implementing
> this. We even have a patch to warn the user when a test case not in any test
> suite is detected:
> lp:~oem-qa/checkbox/patch_display_error_on_whitelist_pattern_not_matched
>
> The patch also warns the user when some of the patterns in the whitelist
> didn't match any job names. In case you would like to consider some of these
> changes in a separate merge proposal, please let me know and I'll create it.

We actually have a bug report asking for checkbox to support suiteless jobs. So it looks like we have a bit of divergence here. I've asked for some input on the bug report and hopefully we can decide what's going to happen with suiteless jobs. However if it's agreed that all jobs will require a suite, those patches might be useful to integrate. I'll let you know when I hear something on the bug.

> > This might be confusing for people creating whitelists manually (us!). I'll
> > try to think of some way to make this more intuitive, but as you mention,
> > keeping things simple would be good.
>
> As I said, I'm open to any suggestions. Thanks.

I'll keep thinking about this, see how we can tackle this problem without complicating the code, if possible.

[lp:~oem-qa/checkbox/patch_selection_dialog_sooner_and_whitelist_ordering]

957. By Launchpad Translations on behalf of checkbox-dev on 2011-07-16

Launchpad automatic translations update.

959. By Javier Collado on 2010-07-13

Dictionary entries removed if set is empty

960. By Javier Collado on 2010-09-01

Files removed from filename cache when they're removed from disk

961. By Sylvain Pineau on 2011-04-18

Add Whitelist ordering support

962. By Sylvain Pineau on 2011-04-18

change code position to avoid merge conflicts

963. By Sylvain Pineau on 2011-04-18

Improve generator expression

[Daniel Manrique (roadmr)]

With the recent cleanup effort on checkbox merge proposals, I just wanted to comment on where these features are going.

I have a prototype implementation of the JobStore that does caching and (IMHO) is less intrusive because it encapsulates the caching mechanism within the JobStore. We can discuss this architecture and decide whether it works as well as the one in this branch. I also thought of another way to implement this that sounds a bit more elegant but could be harder to implement, also to be discussed.

I think the consensus so far is that jobs *need* suites; we always do that and things fail horribly with suiteless jobs, so for the time being we will ignore the "suiteless jobs" bug and proceed under the assumption that jobs need to have suites.

As for the ordering feature, this is also to be discussed, but I think the main blocker for this is the need for the whitelist to be pre-ordered. Marc has done some work with using a stack to order tests, but I think the feature would need to work like this:

- tests are run in whitelist order (best-effort).
- If a job's requirements are listed *after* the job in the whitelist, checkbox overrides the whitelist and puts the requirements *ahead* of the dependent job (but even then, the dependencies *themselves* run in whitelist order). This is to keep users from shooting themselves in the foot.

I think this should be easy enough to implement, and it accomodates both use cases (checkbox-editor helping you, and us certification curmudgeons editing whitelists by hand).

So pending a detailed discussion on these features, I'd appreciate input on whether *this* merge request is still relevant, or we should just discard it since the features will probably be reimplemented pretty much from scratch.

review: Needs Information

[Daniel Manrique (roadmr)]

Checkbox revision 1255 includes these features (run tests in whitelist order and caching jobs in memory for faster gathering), as reworked by Sylvain, Marc and myself. Credit goes to Javier and Sylvain for initially identifying the problem and providing a solution, whose spirit fueled the final implementation.

Take that as an epitaph, as, since the features are already in checkbox trunk, I'll now kill this branch with a bit of remorse but also a great deal of joy.

Enjoy!

review: Disapprove

Unmerged revisions

963. By Sylvain Pineau on 2011-04-18

Improve generator expression

962. By Sylvain Pineau on 2011-04-18

change code position to avoid merge conflicts

961. By Sylvain Pineau on 2011-04-18

Add Whitelist ordering support

960. By Javier Collado on 2010-09-01

Files removed from filename cache when they're removed from disk

959. By Javier Collado on 2010-07-13

Dictionary entries removed if set is empty

958. By Javier Collado on 2010-04-21

Cache variables keep track of name and suite of persisted test cases (LP: #564703 & #541089)

=== modified file 'checkbox/job.py'
--- checkbox/job.py	2010-04-20 16:23:16 +0000
+++ checkbox/job.py	2011-07-22 09:51:14 +0000
@@ -18,6 +18,8 @@
 #
 import os
 import logging
+import re
+from collections import defaultdict
 
 from gettext import gettext as _
 from string import Template
@@ -94,17 +96,88 @@
 
 class JobStore(MessageStore):
     """A job store which stores its jobs in a file system hierarchy."""
+    filename_cache = set()
+    name_cache = defaultdict(set)
+    suite_cache = defaultdict(set)
+
+    def _update_cache(self):
+        """
+        Update name and suite cache
+        """
+        # Read all filenames and create name
+        filenames = [filename
+                     for filename in self._walk_messages()
+                     if filename not in self.filename_cache]
+
+        for filename in filenames:
+            message = self._read_message(filename)
+            name = message.get('name')
+            suite = message.get('suite')
+
+            # Update cache variables
+            self.filename_cache.add(filename)
+            self.name_cache[name].add(filename)
+            self.suite_cache[suite].add(filename)
+
+    def whitelist_ordering(self, whitelist_patterns):
+        """
+        Sort the Message file objects by whitelist patterns
+        """
+
+        # TODO: Apply dependencies
+
+        logging.debug("Whitelist ordering in progress")
+        self._update_cache()
+
+        # msg stored in a the /store filesystem
+        msg = {}
+        # ordered jobs
+        jobs = []
+
+        for filename in self.filename_cache:
+            message = self._read_message(filename)
+            name = message.get("name")
+            os.rename(filename, filename + ".tmp")
+            msg[name] = filename
+
+        for p in whitelist_patterns:
+            job = next((n for n in msg.iterkeys() if p.match(n)), None)
+            if job:
+                jobs.append(job)
+
+        for i, name in enumerate(jobs):
+            #print i, name
+            os.rename(msg[name] + ".tmp", re.sub(r'\d+$', str(i), msg[name]))
+
+        logging.debug("Whitelist ordering completed")
 
     def add(self, job):
+        self._update_cache()
+
         # TODO: Order alphabetically within suite or non-suite
 
         # Remove the same job if it already exists without a suite
-        if "suite" in job:
-            for filename in self._find_matching_messages(name=job["name"], suite=None):
-                os.unlink(filename)
+        if ("suite" in job
+            and job["name"] in self.name_cache
+            and None in self.suite_cache):
+                filenames = list((self.name_cache[job["name"]]
+                                  .intersection(self.suite_cache[None])))
+                for filename in filenames:
+                    os.unlink(filename)
+
+                    # Remove cache entries
+                    if filename in self.filename_cache:
+                        self.filename_cache.remove(filename)
+                    self.name_cache[job["name"]].remove(filename)
+                    if not self.name_cache[job["name"]]:
+                        del self.name_cache[job["name"]]
+                    self.suite_cache[None].remove(filename)
+                    if not self.suite_cache[None]:
+                        del self.suite_cache[None]
+
 
         # Return if the same job is already in the store
-        if list(self._find_matching_messages(name=job["name"])):
+        if job['name'] in self.name_cache:
             return
 
         message_id = super(JobStore, self).add(job)
@@ -112,20 +185,29 @@
         # TODO: Apply dependencies
         if "depends" in job:
             for depends in job["depends"]:
-                for filename in self._find_matching_messages(suite=job.get("suite")):
-                    message = self._read_message(filename)
-                    if job["name"] in message.get("depends", []):
-                        new_filename = self._get_next_message_filename()
-                        os.rename(filename, new_filename)
+                if job.get('suite') in self.suite_cache:
+                    filenames = self.suite_cache[job.get('suite')]
+
+                    for filename in filenames:
+                        message = self._read_message(filename)
+                        if job["name"] in message.get("depends", []):
+                            new_filename = self._get_next_message_filename()
+                            os.rename(filename, new_filename)
+
+                            # Update cache with new filename
+                            self.filename_cache.add(new_filename)
+                            self.name_cache[job["name"]].add(new_filename)
+                            self.suite_cache[job["suite"]].add(new_filename)
+
+                            if filename in self.filename_cache:
+                                self.filename_cache.remove(filename)
+                            if filename in self.name_cache[job["name"]]:
+                                self.name_cache[job["name"]].remove(filename)
+                                if not self.name_cache[job["name"]]:
+                                    del self.name_cache[job["name"]]
+                            if filename in self.suite_cache[job["suite"]]:
+                                self.suite_cache[job["suite"]].remove(filename)
+                                if not self.suite_cache[job["suite"]]:
+                                    del self.suite_cache[job["suite"]]
 
         return message_id
-
-    # TODO: Optimize by only searching backwards until a given condition
-    def _find_matching_messages(self, **kwargs):
-        for filename in self._walk_messages():
-            message = self._read_message(filename)
-            for key, value in kwargs.iteritems():
-                if message.get(key) != value:
-                    break
-            else:
-                yield filename

=== modified file 'plugins/jobs_info.py'
--- plugins/jobs_info.py	2011-07-13 15:40:31 +0000
+++ plugins/jobs_info.py	2011-07-22 09:51:14 +0000
@@ -94,6 +94,9 @@
         def report_message(message):
             self._manager.reactor.fire("report-job", message)
 
+        # Send whitelist info to jobs-prompt for job ordering
+        self._manager.reactor.fire("whitelist", self.whitelist_patterns)
+
         # Set domain and message event handler
         old_domain = gettext.textdomain()
         gettext.textdomain(self.domain)

=== modified file 'plugins/jobs_prompt.py'
--- plugins/jobs_prompt.py	2011-02-02 21:48:35 +0000
+++ plugins/jobs_prompt.py	2011-07-22 09:51:14 +0000
@@ -60,6 +60,8 @@
              ("prompt-job", self.prompt_job),
              ("prompt-jobs", self.prompt_jobs),
              ("prompt-finish", self.prompt_finish),
+             ("jobs-ordering", self.jobs_ordering),
+             ("whitelist", self.report_whitelist),
              ("report", self.report),
              ("report-job", self.report_job)]:
             self._manager.reactor.call_on(rt, rh)
@@ -105,6 +107,13 @@
         if interface.direction == NEXT:
             self.store.delete_all_messages()
 
+    def jobs_ordering(self):
+        if self.whitelist_patterns:
+            self.store.whitelist_ordering(self.whitelist_patterns)
+
+    def report_whitelist(self, whitelist):
+        self.whitelist_patterns = whitelist
+
     def report(self):
         self.store.set_pending_offset(0)
         messages = self.store.get_pending_messages()

=== modified file 'plugins/suites_prompt.py'
--- plugins/suites_prompt.py	2011-02-02 21:48:35 +0000
+++ plugins/suites_prompt.py	2011-07-22 09:51:14 +0000
@@ -78,6 +78,9 @@
                 self._depends[job["name"]] = [job["suite"]]
 
     def prompt_gather(self, interface):
+        # Whitelist ordering
+        self._manager.reactor.fire("jobs-ordering")
+
         # Resolve dependencies
         resolver = Resolver()
         for key in self._jobs.iterkeys():