Expand unit test coverage for UserSettings #1165

Closed
robocoder opened this Issue Feb 20, 2010 · 3 comments

2 participants

@robocoder

Unit tests were added in [1850] but is incomplete. Propose to provide 100% coverage for the user agent strings that Piwik is expected to parse. A large sampling of user agent strings should help in identifying deficiencies in the parser.

For example, the following is mis-identified as Firefox 3.0:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en; rv:1.9.0.10pre) Gecko/2009041800 Camino/2.0b3pre (like Firefox/3.0.10pre)

@mattab
Piwik Open Source Analytics member

I think in an ideal world we could aim for a 100% coverage of user agent strings.

However I am afraid that this will lead to a very complicated user agent parsing code. We need to keep this code simple (to be maintainable) and also fast (as it is executed in the Tracker).

I vote for expanding the unit tests to the 95% top user agents, as well as ensuring accurate detection of mobile devices (iphone VS ipod touch VS blackberry etc.) - if this is not already the case.

But I vote for won't fix on the 100% test coverage that would lead to a complex parsing code which is not necessary for Piwik.

@robocoder

Agreed: 100% test coverage for the browsers that we've already identified in UserAgentParser.php; not 100% of all known browsers in the history of the Internet.

@robocoder

(In [1859]) fixes #1165 - expand UserSettings unit test coverage;
fixes #1167 - fix UserAgentParser issues

@robocoder robocoder added this to the Piwik 0.5.5 milestone Jul 8, 2014
@robocoder robocoder self-assigned this Jul 8, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment