[#MDL-19566] Refactoring of regular expressions and unit test best practice

Issue Details (XML | Word | Printable)

Key:	MDL-19566
Type:	Improvement
Status:	Open
Priority:	Minor
Assignee:	moodle.com
Reporter:	Nicolas Connault
Votes:	0
Watchers:	0

Operations

Add/Edit UI Mockup to this issue

If you were logged in you would be able to see more operations.

Moodle

Refactoring of regular expressions and unit test best practice

Created: 19/Jun/09 05:30 PM Updated: 13/Nov/09 04:08 PM

Return to search

Component/s:

Unit tests

Affects Version/s:

1.9.5

Fix Version/s:

2.0

Participants:	moodle.com, Nicolas Connault and Tim Hunt
Security Level:	None
Affected Branches:	MOODLE_19_STABLE
Fixed Branches:	MOODLE_20_STABLE

Description

« Hide

Regular expressions are very powerful, but most of them in Moodle are untested. Ideally we should unit-test every regular expression with a wide range of inputs.

This is difficult for one main reason: often the regular expressions are not accessible to unit tests, they are deeply embedded in the logic and have strong coupling.

One solution is to extract all static regular expressions into constants, and dynamic expressions into functions. Unit tests can then be written for them easily.

All

Comments

Change History

Version Control

Sort Order:

[ Permalink | « Hide ]

Nicolas Connault added a comment - 22/Jun/09 02:05 PM

I'm changing the requirement of extracting "ALL" regular expressions. I think that only those that are difficult to read need to be unit-tested.

[ Permalink | « Hide ]

Tim Hunt added a comment - 22/Jun/09 02:11 PM

It depends.

I don't think a regular-expression is normally the right level of granularity to test. That is, unit test should test the external API of a unit of code, not the implementation details, and a regex is normally an implementation detail.

So, as a simple example, we should not unit test '/[^A-Za-z0-9]/i', we should unit test clean_param(..., PARAM_ALPHANUM);

Of course, if a regex is perfoming an important task all on its own, and that is currently buried in the middle of a big function, then it might be good to factor out the task done by the regex into a new function.

All content on this web site is made available under the GNU General Public License, unless otherwise stated.