Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29321051: Issue 2715 - Speed up notification target tests (Closed)

Created:
June 24, 2015, 7:06 a.m. by Felix Dahlke
Modified:
Nov. 3, 2015, 7:42 a.m.
Reviewers:
Wladimir Palant
CC:
Thomas Greiner
Visibility:
Public.

Description

Issue 2715 - Speed up notification target tests These tests were pretty heavy. There's some merit in testing all possible combinations, but the tests ran for 40 seconds on my machine. I've made three changes that brought the runtime down to 3 seconds (on my machine): 1. Move target combination tests into a separate test and reduce the number of targets. The target selection test already covers the edge cases of target selection - there is no real need to repeat this for all target combinations. One matching and one non-matching value for each property should suffice. 2. Don't generate pairs of the same target - doesn't really make sense and saves a bit of runtime. 3. Don't test for information vs critical for all target combinations. That critical notifications get precedence is covered by other tests, and not testing it for all target combinations cuts the test runtime in half.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -22 lines) Patch
M chrome/content/tests/notification.js View 2 chunks +21 lines, -22 lines 0 comments Download

Messages

Total messages: 2
Felix Dahlke
June 24, 2015, 7:09 a.m. (2015-06-24 07:09:11 UTC) #1
Wladimir Palant
Nov. 2, 2015, 8:17 p.m. (2015-11-02 20:17:53 UTC) #2
LGTM

Powered by Google App Engine
This is Rietveld