overall, i'm excited about this work, and think it'll be very cool (both for drupal.org, and for other sites i'm working on to manage software development)... On Jun 15, 2006, at 4:52 PM, Rok Žlender wrote:
1. Testing server queries project server for all untested patches. This will require a xml-rpc handler in project module which would generate a list of untested patches or patches marked "patch (needs automated test)" and return it as a response.
given how issues and patches are currently handled, this is a very ambiguous query to attempt to fulfill: a) patches are just files attached to issue comments. there's no enforcement of a given naming convention, etc. it's therefore hard to separate patches worth testing from screenshots and other random attachments of all sorts. b) it's entire issues that are set to a given status, not individual comments. a given issue might have 10+ comments with different patches attached to them, some of which have been reviewed and already vetoed, etc. c) the *last* patch in a given issue isn't always the one people care about. sometimes a patch from early in the thread ends up being the best way to solve something, even if other patches are attached in later comments that attempt other approaches. because of all of this, even marking an issue "patch (needs auto- tests)" doesn't really help much. that'll make it easier to find all the issues that need testing, but not the individual patches. similarly, all this makes the reporting hard, too. when the testing server comes back to post a followup saying "patch foo.txt results: FAILED", that might be comment #45 in a long issue, but foo.txt might be from early in the the thread. it'd be nice to be able to group them more directly. instead of a new follow-up, *maybe* we want to edit the existing comment and add this additional info (though that won't show up as an update in the tracker, etc). i dunno, it's a sticky problem. i haven't thought about all of this long enough to have a proposal for how it should all work. but, i wanted to at least share my concerns so others who have a chance can keep this stuff in mind when thinking about this. some brainstorming... 1) project should use threaded comments, not it's own chronological followups? it'd help to group replies to specific comments more nicely, but it also might be harder to follow. 2) comments should have their own status, in addition to the entire issue? might be handy to deal with existing ambiguities in issues with lots of patches (e.g. comments like "#4 is broken, but #5 is RTBC"). probably too complicated and weird to be practical, but maybe worth considering. 3) we should reconsider how we handle file attachments to issues. instead of associating each patch with a specific comment, we associate all of them with the issue itself (so there's just a growing table of filenames at the top of the issue), and then individual comments refer to filenames when they want to discuss stuff. (there could be an "operations" column in the table, with a "reply" link that brings you to an add follow-up form with some pointer/link to that particular filename automatically filled in for you or something). this table could have metadata associated with each file, so we could clearly mark files as patches, screenshots, etc. maybe each file in this table needs its own status (another field in the metadata), instead of each comment in the issue. the callback from the test server could just add more meta data to specific files, change the status of files, etc. food for thought... thanks, -derek (dww) p.s. unrelated to all of the above, but another potential snag you'll run into: not everyone generates patches the same way. some people do the appropriate, recommended thing, and generate the patch from the root of the drupal source tree. others make them local to a specific directory. now that we're about to move core modules from modules/foo.module to modules/foo/foo.module, patches that just touch a given module would probably be better off from the local directory, so they can apply cleanly to both 4.7 and HEAD (bug fixes, of course). so, your test server is going to have to be very smart about analyzing patch files, trying different things to get them to apply to a given copy of the drupal source. some will just be faulty, bad patches, or patches that no longer apply (which should be reported with a different status than "FAILED TESTS". we probably want (at least) "FAILED TO APPLY", "FAILED TESTS", and "PASSED" as the possible results of a given patch. p.p.s. all that got me thinking of another thing... a given patch might apply just fine today, and pass all tests. however, tomorrow, dries commits a change to HEAD which changes some API and now a) the patch no longer applies and/or b) now starts failing tests. clearly, we need some mechanism/UI for re-testing patches, reporting the history of test attempts for a given patch through time, etc.