Merge lp:~javier.collado/checkbox/bug990075 into lp:checkbox
Status: | Rejected |
---|---|
Rejected by: | Javier Collado |
Proposed branch: | lp:~javier.collado/checkbox/bug990075 |
Merge into: | lp:checkbox |
Diff against target: |
249 lines (+140/-20) (has conflicts) 3 files modified
checkbox/job.py (+0/-15) debian/changelog (+8/-0) plugins/jobs_info.py (+132/-5) Text conflict in debian/changelog Text conflict in plugins/jobs_info.py |
To merge this branch: | bzr merge lp:~javier.collado/checkbox/bug990075 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Javier Collado (community) | Disapprove | ||
Marc Tardif (community) | Needs Fixing | ||
Brendan Donegan (community) | Needs Fixing | ||
Review via email: mp+104369@code.launchpad.net |
Description of the change
This change fixes the job ordering problem found bug990075 as follows:
- Removes the code in JobStore that takes of ordering and duplicate removal
The JobStore was expected to order jobs based on their dependencies when added one by one. This wasn't very efficient because the store had to perform the reordering every time a job was added and it was confusing to the programmer because it wasn't clear the algorithm being used.
- Adds topological sorting to the jobs_info.py plugin
The jobs_info.py plugin contains the whitelist ordering code, so it makes sense to take care of reordering jobs right there also to meet dependencies. Since whitelist ordering might be applied as well, the strategy used is check if the whitelist already provides a valid topological sort. If the check isn't successful, then the sorting is performed once for the whole graph using depth first search before adding any job to the JobStore.
One more thing to consider is that when test suites are added to the job store, "report-message" events are triggered with messages that contain updated information for test cases (like which one is their test suite). Before this fix, a new message was added to the list of messages and the JobStore took care of removing duplicates. Now, since the jobs are already ordered, they are updated instead to preserve their ordering in the jobs_info.py plugin prior to adding jobs to the JobStore.
Unmerged revisions
- 1382. By Javier Collado
-
Fixed dependency ordering check
- 1381. By Javier Collado
-
Added text to make clear what is related to the old/new whitelist
- 1380. By Javier Collado
-
Removed custom code to provide readable output and replaced with difflib
Instead of keeping track of broken dependencies, difflib is used against the
original whitelist and the new whitelist after the job reordering to provide a
diff style output that is much more readable to the user. - 1379. By Javier Collado
-
Updated whitelist error message to display detailed information properly
This change needs the fix for bug1012052 to work fine.
- 1378. By Javier Collado
-
Added message to let the user know what's the final ordering being used
- 1377. By Javier Collado
-
Implemented new algorithm to order jobs
The new algorithm no longer uses a standard topological ordering algorithm
because of the lack of information available in job descriptions. For example,
most of the jobs are expected to be executed before suspend/suspend_ advanced,
but they are not listed as a dependency for this job. Instead, just the jobs
that are expected to be executed after suspend depend on
suspend/suspend_ advanced. To address this problem, it's assumed that the whitelist is already almost
fine, but there are just a few test cases that are out of order (if this is not
the case, the final ordering will probably not be valid). After that, a list of
dependencies and reverse dependencies is created and an attempt to order all
the messages in the whitelist one by one is made. If the order in the whitelist
is fine, then the messsage is added to the list of sorted jobs, if that's not
the case, the message is cached and added as soon as the missing dependencies
have been added as well. - 1376. By Javier Collado
-
Make sure that jobs are topologically ordered (LP: #990075)
- 1375. By Javier Collado
-
Removed unused code to delete duplicates from job store
jobs_info.py plugin now takes care of the duplicates by updating the message
list before any message is added to the store. Hence, the code to remove files
from store for duplicated message is no longer needed. - 1374. By Javier Collado
-
Fixed report-message callback to update existing messages
In the past messages for the same job were added multiple times and it looks
like the store took care of removing duplicates. Now, given that jobs are
already ordered, they can be updated in place before adding them to the store. - 1373. By Javier Collado
-
Added topological sort implementation
Unfortunately, this breaks the selection dialog and the test cases
are no long displayed under their test suites.
One pending thing to check is what happens in the presence of circular dependencies.
Since depth-first search stops when a job has already been inspected, I don't expect
any side effect like and infinite loop.
Looking at the code, it seems that they're already handled in the suites_prompt.py plugin lib.resolver. Resolver, which actually seems to address topological order,
using checkbox.
but I didn't use it because it wasn't used either in the original code (I guess there was
a good reason for that).
Hence, what should happen is that an exception is raised in checkbox. lib.resolver. Resolver
as it would have happened in the past.