Most of you have probably seen the drupal.org front page post concerning the deployment of the automated testing system version 2.1 and the subsequent beta phase of contrib project testing. This post will provide additional details regarding the recent deployment and result of that deployment.
Contributed project testing Without a doubt the biggest new feature provided by the 2.1 update was the addition of the ability to test contributed projects. The reason this took so long to support, was not due to lack of support from the testing master server or testing clients, but rather in the determination of module dependencies. In order to test contrib projects that have that have dependencies on other contrib projects the dependency relationships must be determined so the dependencies can be checked out by the testing clients.
The beta status will continue until the dependecy parsing component is complete and able to determine dependencies with 100% accuracy. I have documented what needs to be done in order to complete the code and will begin working on it, but any help would be appreciated.
The biggest issue that needs to be decided, in part by the community, is the way in which Drupal 7 style dependency information can be provided in Drupal 6 module .info files. Once we have a consensus that does not break compatibility with Drupal 6, core then the system needs to be updated to support Drupal 7.x style dependency restrictions in both 7.x and 6.x compatible projects.
On the positive side of things, we were able to track down issues with contrib testing after enabling reviews on a small number of contrib projects. With the help of the project maintainers reporting issues, I was able to fix the problems relatively quickly. Now that we have deployed the changes I have gone ahead and enabled testing on 31 projects (included Drupal core) that had requested to be included in the beta phase. It is great to see this much interest and enthusiasm by the Drupal community in embracing testing. The following is a list of the 31 projects that are currently taking advantage of the automated quality assurance system.
Brief look at of ATS 2.2
I wrapped a few other bug fixes along with the contrib testing bugs into a new release, which was deployed this morning. You can get a feel for what the contrib testing bugs were that we squashed in the initial testing phase below.
PIFR 6.x-2.2, 2010-01-27
- #695350: Provide ‘last’ field in pifr.retrieve() and correct query.
- Events should only be triggered when trigger modules is available.
- Remove notices when test record is saved before client record.
- #695278: Test list should be generated from root directory.
- #695278: Module ‘tests’ directory should be searched.
- #696044: Patches are not being applied properly to contrib projects.
- #696194: Cannot preview client test information.
- Do not add SimpleTest as dependency if it has already been added.
In an effort to expand Drupal quality assurance beyond straight forward testing, I have written the initial integration for coder and coder_tough_love. Since Drupal core does not currently “pass” the review I have not enabled it on the main Drupal core tests, but have instead provided a stub test to demonstrate the functionality and provide and public set of results to work against.
It is my goal that Drupal 7/8 core and coder/coder_tough_love will be cleaned up to the point where there are no false positives from coder and Drupal itself is clean so that the review can be enabled on all Drupal core patches and commits.
I will queue the stub test for review every few days, or upon request in IRC.
Another new feature provided in the 2.1 update was the addition of a trigger/actions implementation that allows e-mails to be sent from qa.drupal.org upon a test result. The system is not designed to send e-mails to specific project maintainers and such, but instead to notify test client maintainers of a client confirmation failure (client no longer working) and the core maintainers upon a bad commit that break Drupal core.
I would like to extend this system to provide general test e-mails for project maintainers and other interested parties, but that work needs to be done on the drupal.org side of things.
Realizing the system’s power
Something that may not be apparent to everyone is that the pifr/pift system can be used for more then just reviewing patches and commits. Not only can we extend the system to perform performance analysis, test coverage analysis, textual analysis, and automatic tests/security reviews, but we can use the system as a generic platform for performing distributed operations on intelligent triggers.
For example, we could use the system as a distributed platform for generating the API documentation seen on api.drupal.org. The system would automatically rebuild the documentation on commit and due to its distributed nature might even allow for the expansion of api.drupal.org for contrib projects (need to talk to infrastructure).
An advanced plugin API is provided for implementing additional functionality for the system. I have created an easy to extend Drupal specific implementation that should make it easy to build Drupal specific additions. Please contact me in IRC/email or the issue queue if you are interested in developing a plugin.
The future and beyond
I have identified items that I would like to have completed for the next installment of the system, created issues for them, and provided a summary below. I would appreciate any help from the community in completing them.
Project Issue File Review (PIFR) (qa.drupal.org & clients)
- Display "postponed" tests in queue graph somehow
- Standardize and clarify plugin arguments
- Check dependencies recursively when postponing test due to branch failure
- Restrict pifr_server_test_get_since() to the project client making the request
- Listing projects and current status
- Change admin/pifr/server/* to admin/pifr/client/*
- Change 'pifr administer' permission to 'pifr administrator'
- Provide 7.x coder style checks
- Provide mechanism for limiting environment (db) based on environment arguments
Project Issue File Test (PIFT) (drupal.org)
- Provide additional options for testing
- Retest branches automatically on time and commit trigger
- Provide e-mail notification of test results
- Integrate branch tests with project page/releases
- Per project branch filter
- Ensure/Refactor cron_retest() query to only re-test the last file on an issue
- When tests fail set Issue tag to "Failed Testing"
Project Info (drupal.org)
Many of you may be familiar with my previous post Diaries of a Core Developer and that sadly nothing has changed. We had a lot of good discussion in the comments, but as far as I understand nothing has been done to improve the development workflow.
Drupal QA has and is currently having issues due to a complex problem with the core design of SimpleTest. I documented this problem some time ago, and made several attempts to refactor SimpleTest (before and after). The patches were quite large since they did a proper overhaul of the system. I was told to break them into smaller pieces so they would be easier to review, but after discovering how intertwined the code was and not receiving any other real feedback I was burned out after a week of working on the patch (and the lack of time others were willing to spend).
Just as the database layer and other large changes had large patches and lots of follow-up patches, even SimpleTest itself was introduced in a large patch, so does this refactoring. I briefly described what needs to be done and some of the benefits in the original issue. You can find more details on the refactoring in one of my later consolidation issues in which I attempted to explain the design and work through the issue.
As I explained in the issue there are some hackish ways we can work around the problem for the time being. I am happy to implement those fixes, given that I am not wasting my time. I still believe an overall refactoring is in order to fix a number of problems including proper isolation of test processes and a clean way of gather errors.
Before I commit a large amount of time, again, to refactoring the system I would like confirmation that someone will actually review the patch, those involved are fine with the changes, and that we will see this through to be committed (preferably some assurance from Dries of webchick). Of course this leads back the fact that I have no authority as a maintainer to make this crucial decision and that the core maintainers are bogged down reviewing ALL patches, instead of distributing the load to the sub-maintainers and core maintainers reviewing sub-maintainers patches and overall changes.
You may or may not have noticed the launch of the second iteration of the Drupal automated testing system last Thursday. I am very excited to have the system up and running and by the possibilities it opens up.
In order to help others get a feel for some of the new features that it provides and how the features can be used I created a screencast covering some of visual changes and features provided to end-users.
I am posting this in light of webchick’s insightful series of blog posts, a more recent revival of the topic, and my own utter frustration. I understand this subject has been brought up before with no change so I was a bit reluctant to bring it up again, but I think it is important. Personally I have been so fed up with core development that I have taken over a month off from serious core development. Given that other developers express this sentiment from time to time and that you can feel the discontent in the community (granted some are perfectly happy) I think things need to change.
Keep in mind I have reworked this quite a bit over the course of several weeks and sat on the whole subject for some time to ensure that I had my perspective right. Not all my reasons, experiences, and thoughts are included as I tried to focus on the most important.
I am going to present some of my own experiences and my personal mindset when working on Drupal core and the reasons behind it. I will not point to specific issues or people.
General patch workflow/mindset
In general when I submit a patch I expect there to be several revisions of the patch before it is finally committed. I think that is a healthy sign and the patch ends up better at the end. Another thing I expect is the patch to take 1-4 weeks to get committed on average with a few exceptions on both both ends of the spectrum. It usually takes a week or two to get a review and bring it to RTBC status and then anywhere from from a day to three weeks to be committed.
Given that most patches are not standalone, that is to say they are part of a larger set of changes, it can take an extremely long time to get any sort of major change into core. A shortcut to this is the controversial and feared “large patch.” There are two reactions to such a patch. If the patch is a cleanup/refactoring (ie. unexciting) or for whatever whim it may be rejected as “two large to review” and the creator is told to split it into smaller patches. In many cases this is virtually impossible and if possible will take months to get them all the patches committed, not to mention the overload of work to split the patch up. I have let a number of large cleanups of the testing system die for this reason and reworking smaller patches is a joke. To accomplish any major overhaul can easily take 10 patches at ~3 weeks a piece it is impractical. Which means SimpleTest still has issues with static variables, after several attempts, among other things.
There is a more general implication of this, I do not even bother rolling several patches at a time to the same system because they will sit so long that I will be forced to waste time continually re-rolling them. I have attempted this on several occasions hoping I could get things to move a bit faster, but became utterly frustrated. Additionally, I personally ignore minor changes and cleanups, because I do not feel like going through the hassle of getting an obvious patch committed.
Responsibility without authority
Another problem that I have run into is architecture decisions. These decisions tend to result in massive wastes of time from stagnation or constant rewriting. As Larry Garfield noted (excerpt):
Schema API is right now lagging far behind the rest of the DB layer. It’s API is old-school, it has limited capabilities, it’s nearly useless outside of update hooks, and it’s totally out of sync with the rest of the system. In April, I posted an issue laying out two different ways we could take Schema API. I asked “OK, so which do we want to do?” because as Matt says above no one wants to spend dozens of hours on one approach only to be told later “nah, let’s do the other one instead”. You’d charge a client extra to tell you that, but I can’t bill Dries for my core development time. :-)
That was in mid-April.
I personally asked Dries for feedback by email.
I personally asked webchick for feedback in IRC.
I emailed the dev list pointing people to the issue.
I got involved in a lengthy discussion on the dev list, asking “so as DB Maintainer, what the heck do I actually DO? Do I have the authority to just make this decision and not have it overruled on a whim later?”
There are always multiple ways of doing something and many times people do not comment until they see something they do not like. A problem is that in many cases an entire rewrite is required to satisfy them when many times it is just not necessary. Having more people with authority on large decisions will help alleviate this stress.
Now some may say, “So what are you suggesting? Give
moreauthority to the ‘Maintainers’?” People may then look at various core developers, many of whom hold “maintainer” positions and point at the patches that they marked as RTBC that were not. I know I am guilty of this and many other developers as well. My reasoning is that a patch can be debated for centuries before it is marked RTBC and there will still be complaints made so I would rather get those complaints early on rather then later (after waiting a week+ for a review in RTBC queue of course). Better to get my patch’s place in line than debate it before an initial core committer review. I can say for a fact I review my code more thoroughly for my contrib modules since I have commit access to them. I am sure the same would hold true if more core developers were given commit access.
Others may say that it is good to have patches thoroughly reviewed by people who are intimately involved with Drupal (core committers) as they will ensure the patches are better. Granted many reviews I receive in such a manor are very thorough and well thought out, but there are plenty of terrible patches that get by which I can spot flaws with. Everyone is human and no one can catch everything. Then of course I must wait weeks to get the fix in for those patches. Of course the argument above is only valid if you assume a larger commit group means that unqualified developers will review patches. If you take a look at the MAINTAINERS.txt file in core I think you will find that quite the opposite is true.
Another note on the reviews. Many times a patch will improve a system or bit of code dramatically and there are further improvements planned. In the interest of incremental changes (and Extreme Programming) it makes sense to commit the code even if it is not as good as it could be. When writing a follow-up patch one will most likely find shortcomings of the previous patch and clean it up along the way. This process is much faster and much more effective. This allows the majority of code to be better overall instead of just select areas that are reviewed with intense scrutiny.
From what I can see this aged process of core development is holding back Drupal and asking for a fork. It is easily conceivable that enough people will get annoyed (especially as Drupal continues to grow) with the slow speed of development. I do not want to see that happen and I would like to see Drupal get better, faster.
I do not pretend to have all the answers, but I can present the issues I have encountered and logical responses as I see them.
Firstly, regardless of what you look at: patches sitting in RTBC queue, developers sitting for months waiting for a decision, or the fact that a “code slush” is even needed it is clear to see that two committers is not enough. Many people will argue we need more reviewers, but one must look at the RTBC queue and see that would only make things worse. Given that systems like the Schema API are not up-to-date with DBTNG, even though that was introduced very early on in D7 cycle, you know we have issues. I can say for a fact that I was just totally burned out after rewriting the internal browser patch on every whim and as such it is not implemented anywhere in core and I can only imagine that this holds true for numerous systems.
I do not think that adding one, even two committers is the solution. We need a committer for each system (like every other open source project I know and Drupal contrib), who knows that system inside and out. Given that we already have a list of those people, who have no authority at the moment, the fix seems easy to implement. I do not mean that the database maintainer should only have access to /includes/database as that leads to the cases where two lines need to be changed elsewhere or perhaps all modules need to be updated for database API changes and now the maintainer is powerless. Instead maintainers should be watched over by the community and possibly 1-2 overall maintainers, like we have now.
The maintainers then have final say over their respective areas, but should take into consideration discussion with the community and of course the overall maintainers. In the situation Larry described, he would be free to decide on a path and implement it given that no feedback was provided and noone presented any valid reasons against the approach.
Defining some sort of overall roadmap, like what core is to be makes sense and that may fall more in the realm of the overall maintainer(s) and the community. The maintainers are then bound within that roadmap.
I am sure there are additional ways to improve the core development cycle, but I believe the above changes are crucial. If nothing changes, I suspect I will focus on hacking contrib after the D7 bug cycle.
I appreciate thoughts and comments on this issue, but I would appreciate it if only those “actively involved” in Drupal core development would comment as those are the individuals most effected by this.
- the last official backport was April 23 2009
- a number of very cool features have been added in core in addition to incremental changes
- people have been asking
- Drupal 7 is in "code slush"
it seems appropriate to perform another backport. I finished the bulk of the backport during Drupalcon Paris and have been tweaking and fixing bugs from feedback since then.
In order to ensure that all the new features from Drupal 7 were available it was necessary to create a patch against Drupal 6 core which needs to be applied before installation, as described in INSTALL.txt.
Please update and report any issues in the queue and have fun with the new features and proper error reporting!