test(canvas): test text bbox calculator on real canvas (elastic#153)

Removed the jest-canvas-mock package that doesn't use any canvas implementation, substitute the existing test environment with jsdom 14 and updated canvas. Removed an old mock of getContext inside the enzime setup. Increased the scale font size for the bbox calculator and configured few tests for that, to fix the relative issue on chrome. Canvas on node depends on the os, installed library, fonts etc. This commit checks for a maximum difference between the expected value of the measured text of 2px. This commit also revisit the chart_state test suite decoupling each test from each other. fix elastic#152
sebelga · Apr 8, 2019 · f25ef46 · f25ef46
1 parent 3ab9985
commit f25ef46
Show file tree

Hide file tree

Showing 10 changed files with 348 additions and 86 deletions.
diff --git a/.storybook/config.ts b/.storybook/config.ts
@@ -31,6 +31,7 @@ addDecorator(
 );
 
 function loadStories() {
+  require('../stories/playground.tsx');
   require('../stories/bar_chart.tsx');
   require('../stories/line_chart.tsx');
   require('../stories/area_chart.tsx');

diff --git a/jest.config.json b/jest.config.json
@@ -1,11 +1,6 @@
 {
-  "roots": [
-    "<rootDir>/src"
-  ],
+  "roots": ["<rootDir>/src"],
   "preset": "ts-jest",
-  "testEnvironment": "jsdom",
-  "setupFilesAfterEnv": [
-    "<rootDir>/scripts/setup_enzyme.ts",
-    "jest-canvas-mock"
-  ]
-}
+  "testEnvironment": "jest-environment-jsdom-fourteen",
+  "setupFilesAfterEnv": ["<rootDir>/scripts/setup_enzyme.ts"]
+}
diff --git a/package.json b/package.json
@@ -79,7 +79,7 @@
     "@types/storybook__addon-options": "^4.0.1",
     "@types/storybook__react": "^4.0.0",
     "babel-loader": "^8.0.5",
-    "canvas": "^2.3.1",
+    "canvas": "^2.4.1",
     "commitizen": "^3.0.7",
     "css-loader": "^2.1.0",
     "cz-conventional-changelog": "^2.1.0",
@@ -88,7 +88,7 @@
     "enzyme-adapter-react-16": "^1.10.0",
     "husky": "^1.3.1",
     "jest": "^24.1.0",
-    "jest-canvas-mock": "^2.0.0-beta.1",
+    "jest-environment-jsdom-fourteen": "^0.1.0",
     "moment": "^2.24.0",
     "node-sass": "^4.11.0",
     "prettier": "1.16.4",

diff --git a/scripts/setup_enzyme.ts b/scripts/setup_enzyme.ts
@@ -2,5 +2,3 @@ import { configure } from 'enzyme';
 import Adapter from 'enzyme-adapter-react-16';
 
 configure({ adapter: new Adapter() });
-
-HTMLCanvasElement.prototype.getContext = jest.fn();
diff --git a/src/lib/axes/canvas_text_bbox_calculator.test.ts b/src/lib/axes/canvas_text_bbox_calculator.test.ts
@@ -8,15 +8,56 @@ describe('CanvasTextBBoxCalculator', () => {
       width: 0,
       height: 0,
     });
-
-    const expectedDims = {
-      width: 10.6,
-      height: 16,
-    };
-
-    expect(bbox).toEqual(expectedDims);
+    expect(Math.abs(bbox.width - 23.2)).toBeLessThanOrEqual(2);
+    expect(bbox.height).toBe(16);
 
     canvasBboxCalculator.context = null;
     expect(canvasBboxCalculator.compute('foo')).toBe(none);
   });
+  test('can compute near the same width for the same text independently of the scale factor', () => {
+    let canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 5);
+
+    let bbox = canvasBboxCalculator.compute('foo').getOrElse({
+      width: 0,
+      height: 0,
+    });
+    expect(Math.abs(bbox.width - 23.2)).toBeLessThanOrEqual(2);
+    expect(bbox.height).toBe(16);
+
+    canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 10);
+
+    bbox = canvasBboxCalculator.compute('foo').getOrElse({
+      width: 0,
+      height: 0,
+    });
+    expect(Math.abs(bbox.width - 23.2)).toBeLessThanOrEqual(2);
+    expect(bbox.height).toBe(16);
+
+    canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 50);
+
+    bbox = canvasBboxCalculator.compute('foo').getOrElse({
+      width: 0,
+      height: 0,
+    });
+    expect(Math.abs(bbox.width - 23.2)).toBeLessThanOrEqual(2);
+    expect(bbox.height).toBe(16);
+
+    canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 100);
+
+    bbox = canvasBboxCalculator.compute('foo').getOrElse({
+      width: 0,
+      height: 0,
+    });
+    expect(Math.abs(bbox.width - 23.2)).toBeLessThanOrEqual(2);
+    expect(bbox.height).toBe(16);
+
+    canvasBboxCalculator = new CanvasTextBBoxCalculator(undefined, 1000);
+
+    bbox = canvasBboxCalculator.compute('foo').getOrElse({
+      width: 0,
+      height: 0,
+    });
+    expect(Math.abs(bbox.width - 23.2)).toBeLessThanOrEqual(2);
+    expect(bbox.height).toBe(16);
+  });
 });
diff --git a/src/lib/axes/canvas_text_bbox_calculator.ts b/src/lib/axes/canvas_text_bbox_calculator.ts
@@ -7,15 +7,15 @@ export class CanvasTextBBoxCalculator implements BBoxCalculator {
   private offscreenCanvas: HTMLCanvasElement;
   private scaledFontSize: number;
 
-  constructor(rootElement?: HTMLElement) {
+  constructor(rootElement?: HTMLElement, scaledFontSize: number = 100) {
     this.offscreenCanvas = document.createElement('canvas');
     this.offscreenCanvas.style.position = 'absolute';
     this.offscreenCanvas.style.top = '-9999px';
 
     this.context = this.offscreenCanvas.getContext('2d');
     this.attachedRoot = rootElement || document.documentElement;
     this.attachedRoot.appendChild(this.offscreenCanvas);
-    this.scaledFontSize = 5;
+    this.scaledFontSize = scaledFontSize;
   }
   compute(text: string, fontSize = 16, fontFamily = 'Arial', padding: number = 1): Option<BBox> {
     if (!this.context) {

diff --git a/src/state/chart_state.test.ts b/src/state/chart_state.test.ts
@@ -8,6 +8,8 @@ import {
   BarSeriesSpec,
   Position,
 } from '../lib/series/specs';
+import { LIGHT_THEME } from '../lib/themes/light_theme';
+import { mergeWithDefaultTheme } from '../lib/themes/theme';
 import { getAnnotationId, getAxisId, getGroupId, getSpecId } from '../lib/utils/ids';
 import { TooltipType, TooltipValue } from '../lib/utils/interactions';
 import { ScaleBand } from '../lib/utils/scales/scale_band';
@@ -16,7 +18,7 @@ import { ScaleType } from '../lib/utils/scales/scales';
 import { ChartStore } from './chart_state';
 
 describe('Chart Store', () => {
-  const store = new ChartStore();
+  let store = new ChartStore();
 
   const SPEC_ID = getSpecId('spec_1');
   const AXIS_ID = getAxisId('axis_1');
@@ -54,6 +56,11 @@ describe('Chart Store', () => {
       colorValues: [],
     },
   };
+  beforeEach(() => {
+    store = new ChartStore();
+    store.updateParentDimensions(600, 600, 0, 0);
+    store.computeChart();
+  });
 
   test('can add a single spec', () => {
     store.addSeriesSpec(spec);
@@ -70,6 +77,7 @@ describe('Chart Store', () => {
   });
 
   test('can add an axis', () => {
+    store.addSeriesSpec(spec);
     const axisSpec: AxisSpec = {
       id: AXIS_ID,
       groupId: GROUP_ID,
@@ -342,30 +350,42 @@ describe('Chart Store', () => {
       },
     );
 
-    const start = { x: 0, y: 0 };
-    const end1 = { x: 100, y: 0 };
-    const end2 = { x: -100, y: 0 };
-    store.chartDimensions.left = 10;
-
+    const start1 = { x: 0, y: 0 };
+    const start2 = { x: 100, y: 0 };
+    const end1 = { x: 600, y: 0 };
+    const end2 = { x: 300, y: 0 };
+    store.chartTheme = mergeWithDefaultTheme(
+      {
+        chartMargins: {
+          left: 0,
+          right: 0,
+          top: 0,
+          bottom: 0,
+        },
+      },
+      LIGHT_THEME,
+    );
+    store.addSeriesSpec(spec);
+    store.computeChart();
     store.onBrushEndListener = undefined;
     store.onBrushStart();
     expect(store.isBrushing.get()).toBe(false);
-    store.onBrushEnd(start, end1);
+    store.onBrushEnd(start1, end1);
     expect(brushEndListener).not.toBeCalled();
 
     store.setOnBrushEndListener(brushEndListener);
     store.onBrushStart();
     expect(store.isBrushing.get()).toBe(true);
-    store.onBrushEnd(start, start);
+    store.onBrushEnd(start1, start1);
     expect(brushEndListener).not.toBeCalled();
 
-    store.onBrushEnd(start, end1);
-    expect(brushEndListener.mock.calls[0][0]).toBeCloseTo(0.9, 1);
-    expect(brushEndListener.mock.calls[0][1]).toBeCloseTo(1.5, 1);
+    store.onBrushEnd(start1, end1);
+    expect(brushEndListener.mock.calls[0][0]).toBe(1);
+    expect(brushEndListener.mock.calls[0][1]).toBe(4);
 
-    store.onBrushEnd(start, end2);
-    expect(brushEndListener.mock.calls[1][0]).toBeCloseTo(0.3, 1);
-    expect(brushEndListener.mock.calls[1][1]).toBeCloseTo(0.9, 1);
+    store.onBrushEnd(start2, end2);
+    expect(brushEndListener.mock.calls[1][0]).toBe(1.5);
+    expect(brushEndListener.mock.calls[1][1]).toBe(2.5);
   });
 
   test('can update parent dimensions', () => {
@@ -604,6 +624,8 @@ describe('Chart Store', () => {
   });
 
   test('can disable tooltip on brushing', () => {
+    store.addSeriesSpec(spec);
+    store.setOnBrushEndListener(() => ({}));
     const tooltipValue: TooltipValue = {
       name: 'a',
       value: 'a',
@@ -613,15 +635,15 @@ describe('Chart Store', () => {
     };
     store.xScale = new ScaleContinuous([0, 100], [0, 100], ScaleType.Linear);
     store.cursorPosition.x = 1;
-    store.cursorPosition.x = 1;
+    store.cursorPosition.y = 1;
     store.tooltipType.set(TooltipType.Crosshairs);
     store.tooltipData.replace([tooltipValue]);
     store.onBrushStart();
     expect(store.isBrushing.get()).toBe(true);
     expect(store.isTooltipVisible.get()).toBe(false);
 
     store.cursorPosition.x = 1;
-    store.cursorPosition.x = 1;
+    store.cursorPosition.y = 1;
     store.tooltipType.set(TooltipType.Crosshairs);
     store.tooltipData.replace([tooltipValue]);
     store.onBrushEnd({ x: 0, y: 0 }, { x: 1, y: 1 });

diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts
@@ -743,10 +743,10 @@ export class ChartStore {
     this.customSeriesColors = new Map([...this.customSeriesColors, ...updatedCustomSeriesColors]);
 
     // tslint:disable-next-line:no-console
-    // console.log({colors: seriesDomains.seriesColors});
+    console.log({ colors: seriesDomains.seriesColors });
 
     // tslint:disable-next-line:no-console
-    // console.log({ seriesDomains });
+    console.log({ seriesDomains });
     this.seriesColorMap = getSeriesColorMap(
       seriesDomains.seriesColors,
       this.chartTheme.colors,
@@ -761,7 +761,7 @@ export class ChartStore {
       this.deselectedDataSeries,
     );
     // tslint:disable-next-line:no-console
-    // console.log({ legendItems: this.legendItems });
+    console.log({ legendItems: this.legendItems });
 
     const {
       xDomain,
@@ -822,7 +822,7 @@ export class ChartStore {
     );
 
     // tslint:disable-next-line:no-console
-    // console.log({ seriesGeometries });
+    console.log({ seriesGeometries });
     this.geometries = seriesGeometries.geometries;
     this.xScale = seriesGeometries.scales.xScale;
     this.yScales = seriesGeometries.scales.yScales;

diff --git a/stories/interactions.tsx b/stories/interactions.tsx
@@ -449,6 +449,47 @@ storiesOf('Interactions', module)
       </Chart>
     );
   })
+  .add('brush selection tool on bar chart linear', () => {
+    return (
+      <Chart renderer="canvas" className={'story-chart'}>
+        <Settings onBrushEnd={action('onBrushEnd')} />
+        <Axis
+          id={getAxisId('bottom')}
+          position={Position.Bottom}
+          title={'bottom'}
+          showOverlappingTicks={true}
+        />
+        <Axis
+          id={getAxisId('left')}
+          title={'left'}
+          position={Position.Left}
+          tickFormat={(d) => Number(d).toFixed(2)}
+        />
+        <Axis
+          id={getAxisId('top')}
+          position={Position.Top}
+          title={'top'}
+          showOverlappingTicks={true}
+        />
+        <Axis
+          id={getAxisId('right')}
+          title={'right'}
+          position={Position.Right}
+          tickFormat={(d) => Number(d).toFixed(2)}
+        />
+
+        <BarSeries
+          id={getSpecId('lines')}
+          xScaleType={ScaleType.Linear}
+          yScaleType={ScaleType.Linear}
+          xAccessor="x"
+          yAccessors={['y']}
+          data={[{ x: 1, y: 2 }, { x: 2, y: 7 }, { x: 3, y: 3 }]}
+          yScaleToDataExtent={false}
+        />
+      </Chart>
+    );
+  })
   .add('brush selection tool on time charts', () => {
     const now = DateTime.fromISO('2019-01-11T00:00:00.000Z').toMillis();
     const oneDay = 1000 * 60 * 60 * 24;
Original file line number	Diff line number	Diff line change
Expand Up		@@ -2,5 +2,3 @@ import { configure } from 'enzyme';
		import Adapter from 'enzyme-adapter-react-16';

		configure({ adapter: new Adapter() });

		HTMLCanvasElement.prototype.getContext = jest.fn();