Opened 4 years ago

Closed 4 years ago

#1165 closed Bug (fixed)

Expand unit test coverage for UserSettings

Reported by: vipsoft Owned by: vipsoft
Priority: normal Milestone: Piwik 0.5.5
Component: Core Keywords:
Cc: Sensitive: no


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: Gecko/2009041800 Camino/2.0b3pre (like Firefox/3.0.10pre)

Change History (3)

comment:1 Changed 4 years ago by matt (mattab)

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.

comment:2 Changed 4 years ago by vipsoft (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.

comment:3 Changed 4 years ago by vipsoft (robocoder)

  • Resolution set to fixed
  • Status changed from new to closed

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

Note: See TracTickets for help on using tickets.