Skip to content

Commit

Permalink
Rewrite unit test waitForResize helper (chartjs#4566)
Browse files Browse the repository at this point in the history
The original implementation tries to intercept events from the chart internal iframe, which ones failing on Chrome 60. Checking internals doesn't seem the best approach, instead we could consider that a chart has been resized after the resize method has been called and processed. So let's hook `Chart.resize` and callback once it's done.
  • Loading branch information
simonbrunel authored Jul 26, 2017
1 parent 1a9bf75 commit 8ba0b15
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 33 deletions.
7 changes: 1 addition & 6 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,8 @@ sudo: required
dist: trusty

addons:
chrome: stable
firefox: latest
apt:
sources:
- google-chrome
packages:
- google-chrome-stable


# IMPORTANT: scripts require GITHUB_AUTH_TOKEN and GITHUB_AUTH_EMAIL environment variables
# IMPORTANT: scripts has to be set executables in the Git repository (error 127)
Expand Down
1 change: 1 addition & 0 deletions test/.eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ globals:
acquireChart: true
Chart: true
moment: true
waitForResize: true

# http://eslint.org/docs/rules/
rules:
Expand Down
1 change: 1 addition & 0 deletions test/jasmine.index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ var utils = require('./jasmine.utils');

window.acquireChart = acquireChart;
window.releaseChart = releaseChart;
window.waitForResize = utils.waitForResize;
window.createMockContext = createMockContext;

// some style initialization to limit differences between browsers across different plateforms.
Expand Down
12 changes: 11 additions & 1 deletion test/jasmine.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,10 +149,20 @@ function specsFromFixtures(path) {
};
}

function waitForResize(chart, callback) {
var override = chart.resize;
chart.resize = function() {
chart.resize = override;
override.apply(this, arguments);
callback();
};
}

module.exports = {
injectCSS: injectCSS,
createCanvas: createCanvas,
acquireChart: acquireChart,
releaseChart: releaseChart,
specsFromFixtures: specsFromFixtures
specsFromFixtures: specsFromFixtures,
waitForResize: waitForResize
};
13 changes: 0 additions & 13 deletions test/specs/core.controller.tests.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,5 @@
describe('Chart', function() {

function waitForResize(chart, callback) {
var resizer = chart.canvas.parentNode._chartjs.resizer;
var content = resizer.contentWindow || resizer;
var state = content.document.readyState || 'complete';
var handler = function() {
Chart.helpers.removeEvent(content, 'load', handler);
Chart.helpers.removeEvent(content, 'resize', handler);
setTimeout(callback, 50);
};

Chart.helpers.addEvent(content, state !== 'complete' ? 'load' : 'resize', handler);
}

// https://github.com/chartjs/Chart.js/issues/2481
// See global.deprecations.tests.js for backward compatibility
it('should be defined and prototype of chart instances', function() {
Expand Down
13 changes: 0 additions & 13 deletions test/specs/platform.dom.tests.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,5 @@
describe('Platform.dom', function() {

function waitForResize(chart, callback) {
var resizer = chart.canvas.parentNode._chartjs.resizer;
var content = resizer.contentWindow || resizer;
var state = content.document.readyState || 'complete';
var handler = function() {
Chart.helpers.removeEvent(content, 'load', handler);
Chart.helpers.removeEvent(content, 'resize', handler);
setTimeout(callback, 50);
};

Chart.helpers.addEvent(content, state !== 'complete' ? 'load' : 'resize', handler);
}

describe('context acquisition', function() {
var canvasId = 'chartjs-canvas';

Expand Down

0 comments on commit 8ba0b15

Please sign in to comment.