The future of quality and testing in ownCloud
At the last ownCloud sprint in Berlin we ran a workgroup on QA and testing. In my capacity as an ownCloud systems developer, myself and four others spent a day working on how to improve the quality and stability of ownCloud in future, with a particular focus on tools and automation of checks and reviews.
The result was a set of proposals which I presented to sprint attendees on the last evening. Only 24 hours after we had come up with these ideas, some had already been implemented, and by the end of the sprint, one of the most important ones — a peer-review based work-flow — had been agreed and adopted. This is testament to the benefits of centralised sprints – with so many important people in the same room we were able to develop ideas, share them, and even execute some of them in a single weekend!
Below are the original proposals made at the sprint. Look out for status indicators like [DONE] and [IN PROGRESS]. Please note that these are just proposals, and not an agreed roadmap. Some of the ideas are good and have been implemented, but others may not be so good, and may not be implemented.
Share your own views either in the comments or on the ownCloud public mailing list.
Proposals
- Issue tracker management
- New feature introduction
- Isolation of functionality and apps
- Bug fixing workflow
- Release procedure
- Coding standards
Issue tracker management
- We can Improve communication of where to report bugs
- Add a link to the GitHub issue tracker in main nav-bar and/or at top of developer homepage nav
- Remove “bug reports” and “feature requests” section of the forum [IN PROGRESS]
- Identify and consolidate contact platforms and support them equally well, or remove
- We can make reported bugs more useful
- Define labels on github issues [DONE]
- Use issue milestones
- Publish who maintains which apps/features (see New feature introduction)
- Formalise bug triaging process – consider appointment of an official bug quality controller
New feature introduction
- App / feature mini-specifications
- Adopt convention of writing explanations of key features / goals of apps
- Contain a few paragraphs or bullet points only
- Include them in info.xml using
<spec>tag? - This way other developers can spot high-level opportunities and issues early on
- Implement a code review system
- Short term: use GitHub’s pull request system for all commits to ‘Stable’ and ‘Master’ branches, perform code reviews before changes are merged, and integrate the ownCloud CI server into the pull request workflow [DONE]
- Long term: consider using a stand-alone web-based code review system like Gerrit
-
Potential benefits:
- Ensure all changes are checked by humans before
commit (chain of responsibility) - ‘Stable’ branch doesn’t get broken
- Integrated peer review
- Integrated automated test suites
- More open governance
- A cleaner git history (easier identification of
issue origin) - Easy extendability of checks (security)
- Automated code quality checks
- Specifically, by integrating ownCloud’s existing JenkinsCI server into the process we can:
- Ensure all changes pass existing automated tests before commit (PHPUnit, Cucumber, others)
- Define additional tests for common issues (code sniffers, correct use of API etc.)
- Check for security problems (e.g. error_log(), file upload, echo in templates)
- Give contributors automated feedback before another human has to review it (simple errors are automatically spotted and notifications sent)
- Ensure all changes are checked by humans before
Isolation of functionality and apps
- Long term goal:
- Move towards object-oriented code, away from static methods (see coding standards)
- Short term goal:
- Make internals more independently testable by decoupling static dependencies (dependency injection) [DONE]
- Abstract away static method calls one level to allow the use of alternatives in unit tests (mocking objects )
- Instead of
OC_User::getUser('foo');use$api->OC_User->getUser('foo'); -
Potential benefits:
- Enables isolation of functionality under test
- Doesn’t affect existing code (usage is voluntary)
- Supports multiple versions of internal ownCloud API (provides a compatibility layer between apps etc.)
- Very simple to use (just add
$api->before existing class names)
Bugfixing workflow
- Proposed bug-fixing procedure (for PHP):
- Reproduce the bug
- Write a unit test which isolates the behaviour
- Check the test fails and confirms the issue
- Fix the bug
- Check the unit test now passes
- Mark bug fixed
Release procedure, security announcements
- Promote the security list, push public to use it instead of bugtracker via website links
- Ensure release procedure includes checking CI server test status
- Expand existing unit tests (see Isolation of functionality and apps & New feature introduction)
- Ensure all necessary apps are enabled on CI server for tests leading up to new release
Coding standards (link)
- Add code-sniffers to CI to identify problematic code (see New feature introduction)
- Add recommendation for object oriented code (see Isolation of functionality and apps)
- Add requirement of mini-spec files (see New feature introduction)
- Add requirement of DocBlock comments for all classes and methods
- Disallow use of dangerous functions in templates [DONE]
- Add policy on word order of “public static function”
