From 493c34d36fcfc99f1a8c190ad4581540f454c24f Mon Sep 17 00:00:00 2001 From: Tim 'mithro' Ansell Date: Mon, 16 Dec 2013 17:09:32 +1100 Subject: [PATCH 1/2] Rework property getting code in _assert_style_element. * Makes the code significantly easier to read. * Reduces duplicate code. * Runs through get_computed_style better. --- test/bootstrap.js | 119 +++++++++++------- test/testcases/auto-test-box-shadow-checks.js | 38 +++--- test/testcases/unit-test-testharness.html | 2 +- 3 files changed, 91 insertions(+), 68 deletions(-) diff --git a/test/bootstrap.js b/test/bootstrap.js index fa953864..4b59a10f 100644 --- a/test/bootstrap.js +++ b/test/bootstrap.js @@ -312,6 +312,54 @@ function _assert_important_in_array(actual, expected, message) { } window.assert_styles_assert_important_in_array = _assert_important_in_array; +/** + * Optionally sets and then gets a property value from a given element. + * + * @param {String} name The name of the property to get + * @param {Element} element DOM node to get the property from + * @param {Object} value DOM node to get the property from + * + * @private + */ +function _set_get_property(name, element, value) { + var set = typeof value != "undefined"; + + // ctm is special + if (name == 'ctm') { + if (set) { + var values = _extract_important(value); + var s = "matrix(" + values[0] + "," + values[1] + "," + values[2] + "," + values[3] + "," + values[4] + "," + values[5] + ")"; + element.setAttribute("transform", s); + } + + var ctm = element.getCTM(); + return '{' + ctm.a + ', ' + + ctm.b + ', ' + ctm.c + ', ' + ctm.d + ', ' + + ctm.e + ', ' + ctm.f + '}'; + } + + if (is_svg_attrib(name, element)) { + if (set) { + element.setAttribute(name, value); + } + return element.attributes[name].value; + } else { + // Map transform to the browser specific value. + if (name == 'transform') { + name = test_features.transformProperty; + } + + if (set) { + element.style[name] = ""; + element.style[name] = value; + if (element.style[name] == "") { + return undefined; + } + } + return getComputedStyle(element, null).getPropertyValue(name); + } +} + /** * asserts that actual has the same styles as the dictionary given by * expected. @@ -350,65 +398,40 @@ function _assert_style_element(object, style, description) { for (var prop_name in style) { // If the passed in value is an element then grab its current style for // that property + var prop_values = []; if (style[prop_name] instanceof HTMLElement || style[prop_name] instanceof SVGElement) { - var prop_value = getComputedStyle(style[prop_name], null)[prop_name]; - } else { - var prop_value = style[prop_name]; - } - - prop_value = '' + prop_value; - - if (prop_name == 'transform') { - var output_prop_name = test_features.transformProperty; + prop_values.push(_set_get_property(prop_name, style[prop_name], undefined)); } else { - var output_prop_name = prop_name; + if (style[prop_name] instanceof Array) { + prop_values.push.apply(prop_values, style[prop_name]); + } else { + prop_values.push(style[prop_name]); + } } - var is_svg = is_svg_attrib(prop_name, object); - if (is_svg) { - reference_element.setAttribute(prop_name, prop_value); - - var current_style = object.attributes; - var target_style = reference_element.attributes; - } else { - reference_element.style[output_prop_name] = prop_value; - - var current_style = computedObjectStyle; - var target_style = getComputedStyle(reference_element, null); - - _assert_important_in_array( - prop_value, [reference_element.style[output_prop_name], target_style[output_prop_name]], - 'Tried to set the reference element\'s '+ output_prop_name + - ' to ' + JSON.stringify(prop_value) + - ' but neither the style' + - ' ' + JSON.stringify(reference_element.style[output_prop_name]) + - ' nor computedStyle ' + JSON.stringify(target) + - ' ended up matching requested value.'); + // Send all the values via getComputedStyle / SVGElement + var expected_prop_values = []; + var errors = []; + for (var i = 0; i < prop_values.length; i++) { + var reference_prop_value = _set_get_property(prop_name, reference_element, prop_values[i]); + if (typeof reference_prop_value != "undefined" && expected_prop_values.indexOf(reference_prop_value) < 0) { + expected_prop_values.push(reference_prop_value); + } else { + errors.push(' Unable to set value to "' + prop_values[i] + '"\n'); + } } - if (prop_name == 'ctm') { - var ctm = object.getCTM(); - var curr = '{' + ctm.a + ', ' + - ctm.b + ', ' + ctm.c + ', ' + ctm.d + ', ' + - ctm.e + ', ' + ctm.f + '}'; - - var target = prop_value; - - } else if (is_svg) { - var target = target_style[prop_name].value; - var curr = current_style[prop_name].value; - } else { - var target = target_style[output_prop_name]; - var curr = current_style[output_prop_name]; + if (expected_prop_values.length == 0) { + throw new AssertionError( + 'Tried to set the reference element\'s "' + prop_name + '"' + + ' but all values where invalid:\n' + errors.join('\n')); } - var description_extra = '\n Property ' + prop_name; - if (prop_name != output_prop_name) - description_extra += '(actually ' + output_prop_name + ')'; + var actual = _set_get_property(prop_name, object, undefined); - _assert_important_in_array(curr, [target], description + description_extra); + _assert_important_in_array(actual, expected_prop_values, 'Checking '+ prop_name); } } finally { if (reference_element.parentNode) { diff --git a/test/testcases/auto-test-box-shadow-checks.js b/test/testcases/auto-test-box-shadow-checks.js index e72c2a9f..4b4eecfb 100644 --- a/test/testcases/auto-test-box-shadow-checks.js +++ b/test/testcases/auto-test-box-shadow-checks.js @@ -1,42 +1,42 @@ timing_test(function() { at(0, function() { assert_styles(".anim", [ - {'boxShadow':'rgb(0, 0, 255) -20px -20px 0px 0px'}, - {'boxShadow':'rgb(0, 0, 255) -20px -20px 8px 0px inset'}, - {'boxShadow':'rgb(0, 0, 255) 20px 20px 8px 0px inset'}, + {'boxShadow': ['rgb(0, 0, 255) -20px -20px 0px 0px', '-20px -20px 0px 0px rgb(0, 0, 255)']}, + {'boxShadow': ['rgb(0, 0, 255) -20px -20px 8px 0px inset', 'inset -20px -20px 8px 0px rgb(0, 0, 255)']}, + {'boxShadow': ['rgb(0, 0, 255) 20px 20px 8px 0px inset', 'inset rgb(0, 0, 255) 20px 20px 8px 0px']}, {'boxShadow':'none'}, ]); }); at(1, function() { assert_styles(".anim", [ - {'boxShadow':'rgb(0, 32, 191) -10px -10px 3px 2px'}, - {'boxShadow':'rgb(0, 32, 191) -10px -10px 9px 2px inset'}, - {'boxShadow':'rgb(0, 0, 255) 20px 20px 8px 0px inset'}, + {'boxShadow': ['rgb(0, 32, 191) -10px -10px 3px 2px', '-10px -10px 3px 2px rgb(0, 32, 191)']}, + {'boxShadow': ['rgb(0, 32, 191) -10px -10px 9px 2px inset', 'inset -10px -10px 9px 2px rgb(0, 32, 191)']}, + {'boxShadow': ['rgb(0, 0, 255) 20px 20px 8px 0px inset', 'inset 20px 20px 8px 0px rgb(0, 0, 255)']}, {'boxShadow':'none'}, ]); }); at(2, function() { assert_styles(".anim", [ - {'boxShadow':'rgb(0, 64, 128) 0px 0px 6px 4px'}, - {'boxShadow':'rgb(0, 64, 128) 0px 0px 10px 4px inset'}, - {'boxShadow':'rgb(0, 128, 0) 20px 20px 12px 8px'}, - {'boxShadow':'rgb(0, 128, 0) 20px 20px 12px 8px'}, + {'boxShadow': ['rgb(0, 64, 128) 0px 0px 6px 4px', '0px 0px 6px 4px rgb(0, 64, 128)']}, + {'boxShadow': ['rgb(0, 64, 128) 0px 0px 10px 4px inset', 'inset 0px 0px 10px 4px rgb(0, 64, 128)']}, + {'boxShadow': ['rgb(0, 128, 0) 20px 20px 12px 8px', '20px 20px 12px 8px rgb(0, 128, 0)']}, + {'boxShadow': ['rgb(0, 128, 0) 20px 20px 12px 8px', '20px 20px 12px 8px rgb(0, 128, 0)']}, ]); }); at(3, function() { assert_styles(".anim", [ - {'boxShadow':'rgb(0, 96, 64) 10px 10px 9px 6px'}, - {'boxShadow':'rgb(0, 96, 64) 10px 10px 11px 6px inset'}, - {'boxShadow':'rgb(0, 128, 0) 20px 20px 12px 8px'}, - {'boxShadow':'rgb(0, 128, 0) 20px 20px 12px 8px'}, + {'boxShadow': ['rgb(0, 96, 64) 10px 10px 9px 6px', '10px 10px 9px 6px rgb(0, 96, 64)']}, + {'boxShadow': ['rgb(0, 96, 64) 10px 10px 11px 6px inset', 'inset 10px 10px 11px 6px rgb(0, 96, 64)']}, + {'boxShadow': ['rgb(0, 128, 0) 20px 20px 12px 8px', '20px 20px 12px 8px rgb(0, 128, 0)']}, + {'boxShadow': ['rgb(0, 128, 0) 20px 20px 12px 8px', '20px 20px 12px 8px rgb(0, 128, 0)']}, ]); }); at(4, function() { assert_styles(".anim", [ - {'boxShadow':'rgb(0, 128, 0) 20px 20px 12px 8px'}, - {'boxShadow':'rgb(0, 128, 0) 20px 20px 12px 8px inset'}, - {'boxShadow':'rgb(0, 128, 0) 20px 20px 12px 8px'}, - {'boxShadow':'rgb(0, 128, 0) 20px 20px 12px 8px'}, + {'boxShadow': ['rgb(0, 128, 0) 20px 20px 12px 8px', '20px 20px 12px 8px rgb(0, 128, 0)']}, + {'boxShadow': ['rgb(0, 128, 0) 20px 20px 12px 8px inset', 'inset 20px 20px 12px 8px rgb(0, 128, 0)']}, + {'boxShadow': ['rgb(0, 128, 0) 20px 20px 12px 8px', '20px 20px 12px 8px rgb(0, 128, 0)']}, + {'boxShadow': ['rgb(0, 128, 0) 20px 20px 12px 8px', '20px 20px 12px 8px rgb(0, 128, 0)']}, ]); }); -}, "Auto generated tests"); \ No newline at end of file +}, "Auto generated tests"); diff --git a/test/testcases/unit-test-testharness.html b/test/testcases/unit-test-testharness.html index b863e727..690e55aa 100644 --- a/test/testcases/unit-test-testharness.html +++ b/test/testcases/unit-test-testharness.html @@ -47,7 +47,7 @@ assert_styles(element, {'left': 'abc123'}, "Description 1"); assert_unreached(); } catch (e) { - assert_regexp_match(e.message, /^Tried to set the reference element\'s left to "abc123" but neither/); + assert_regexp_match(e.message, /^Tried to set the reference element\'s "left"/); } try { From 6fb140e84e5ef312094760b354b818fc742799b7 Mon Sep 17 00:00:00 2001 From: Tim 'mithro' Ansell Date: Tue, 3 Jun 2014 15:35:52 +1000 Subject: [PATCH 2/2] Removing --strict so lint only enforces "Google Javascript Style" rather then "Closure Javascript Style". --- run-lint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-lint.sh b/run-lint.sh index 70abb68c..832a30ab 100755 --- a/run-lint.sh +++ b/run-lint.sh @@ -15,5 +15,5 @@ fi # Comment out the (function() {} ) wrapper sed -e'17s-^-//-' -e'$s-^-//-' web-animations.js > web-animations-4lint.js -gjslint --summary --nojsdoc --strict web-animations-4lint.js +gjslint --summary --nojsdoc web-animations-4lint.js exit $?