Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: fit functions for null y1 values #416

Merged
merged 28 commits into from
Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a8981d4
feat: fit functions for null y1 values
nickofthyme Oct 11, 2019
6332ec0
fix: nearest fit and add zero fit
nickofthyme Oct 11, 2019
c9fd456
fix: cleanup logic
nickofthyme Oct 11, 2019
d4a2455
test: fix broken tests
nickofthyme Oct 11, 2019
87041fb
feat: add story docs and usage case
nickofthyme Oct 11, 2019
8b02916
test: update url encoding, add missing screenshots
nickofthyme Oct 14, 2019
f270306
WIP
nickofthyme Oct 14, 2019
a7e1fe5
feat: Add fitted area clipping logic, update story, add end value fal…
nickofthyme Oct 15, 2019
e397a22
WIP
nickofthyme Oct 16, 2019
0b8f73a
fix: clippedRanges logic
nickofthyme Oct 17, 2019
09a9665
feat: add clipping to lines, update story
nickofthyme Oct 18, 2019
b3566cc
test: add vrt for fitting functions
nickofthyme Oct 18, 2019
931aa00
WIP
nickofthyme Oct 21, 2019
2624421
Merge branch 'master' into feat/fit-functions
nickofthyme Oct 22, 2019
f4332a1
WIP
nickofthyme Oct 23, 2019
448460b
feat: add nearest endValue logic
nickofthyme Oct 23, 2019
f7452b8
test: add tests and test helpers for fit function logic
nickofthyme Oct 23, 2019
1dbae94
Merge branch 'master' into feat/fit-functions
nickofthyme Oct 23, 2019
9911881
fix: quick code cleanup
nickofthyme Oct 23, 2019
81db152
feat: add support for ordinal scales
nickofthyme Oct 24, 2019
daf9385
test: add/fix tests for ordinal scale fitting functions
nickofthyme Oct 24, 2019
f10ed39
test: add more tests around clipping logic
nickofthyme Oct 28, 2019
112cbf4
Merge branch 'master' into feat/fit-functions
nickofthyme Oct 28, 2019
b491e32
test: fix VRT with change in storybook story
nickofthyme Oct 28, 2019
615af36
fix: folder directory
nickofthyme Nov 1, 2019
07031d0
Merge branch 'master' into feat/fit-functions
nickofthyme Nov 1, 2019
4d990b2
fix: pr comments
nickofthyme Nov 13, 2019
ac9f744
Merge branch 'master' into feat/fit-functions
nickofthyme Nov 13, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ module.exports = {
},
],
'sort-keys': 'off',
'import/no-default-export': 'error',
'import/no-unresolved': 'error',
'no-irregular-whitespace': 'error',
'no-unused-expressions': 'error',
Expand All @@ -78,7 +77,7 @@ module.exports = {
},
overrides: [
{
files: ['*.js'],
files: ['*.js', '*test.ts'],
rules: {
'@typescript-eslint/no-var-requires': 0,
},
Expand Down
88 changes: 58 additions & 30 deletions .playground/playgroud.tsx
Original file line number Diff line number Diff line change
@@ -1,38 +1,66 @@
import React from 'react';
import {
Axis,
Chart,
getAxisId,
getSpecId,
Position,
ScaleType,
HistogramBarSeries,
DARK_THEME,
Settings,
} from '../src';
import { KIBANA_METRICS } from '../src/utils/data_samples/test_dataset_kibana';
import { Axis, Chart, getAxisId, getSpecId, Position, ScaleType, Settings, LineSeries } from '../src';
import { Fit } from '../src/chart_types/xy_chart/utils/specs';

const data = [
{ x: 0, y: null },
{ x: 1, y: 3 },
{ x: 2, y: 5 },
{ x: 3, y: null },
{ x: 4, y: 4 },
{ x: 5, y: null },
{ x: 6, y: 5 },
{ x: 7, y: 6 },
{ x: 8, y: null },
{ x: 9, y: null },
{ x: 10, y: null },
{ x: 11, y: 12 },
{ x: 12, y: null },
];

export class Playground extends React.Component {
render() {
const data = KIBANA_METRICS.metrics.kibana_os_load[0].data.slice(0, 5);
return (
<div className="chart">
<Chart>
<Settings theme={DARK_THEME} rotation={180} />
<Axis id={getAxisId('x')} position={Position.Bottom} />
<Axis id={getAxisId('y')} position={Position.Left} />

<HistogramBarSeries
id={getSpecId('series bars chart')}
xScaleType={ScaleType.Linear}
yScaleType={ScaleType.Linear}
xAccessor={0}
yAccessors={[1]}
data={data}
yScaleToDataExtent={true}
/>
</Chart>
</div>
<>
<div className="chart">
<Chart className="story-chart">
<Settings
showLegend
theme={{
areaSeriesStyle: {
point: {
visible: true,
},
},
}}
/>
<Axis
id={getAxisId('bottom')}
position={Position.Bottom}
title={'Bottom axis'}
showOverlappingTicks={true}
/>
<Axis id={getAxisId('left')} title={'Left axis'} position={Position.Left} />
<LineSeries
id={getSpecId('test')}
xScaleType={ScaleType.Linear}
yScaleType={ScaleType.Linear}
xAccessor={'x'}
yAccessors={['y']}
// curve={2}
// splitSeriesAccessors={['g']}
// stackAccessors={['x']}
fit={Fit.Linear}
data={data}
// fit={{
// type: Fit.Average,
// endValue: 0,
// }}
// data={data}
/>
</Chart>
</div>
</>
);
}
}
2 changes: 2 additions & 0 deletions global.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// https://github.com/jest-community/jest-extended
import 'jest-extended';
1 change: 1 addition & 0 deletions integration/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ function requireAllStories() {
function encodeString(string: string) {
return string
.replace(/\//gi, ' ')
.replace(/-/g, ' ')
.replace(/[^a-z|A-Z|0-9|\s|\/]+/gi, '')
.trim()
.replace(/\s+/g, '-')
Expand Down
2 changes: 1 addition & 1 deletion integration/page_objects/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class CommonPage {

expect(chart).toMatchImageSnapshot();
} catch (error) {
throw new Error(error);
throw new Error(`${error}\n\n${url}`);
}
}
async loadChartFromURL(url: string) {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion integration/tests/all.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ describe('Baseline Visual tests for all stories', () => {
stories.forEach(({ title, encodedTitle }) => {
describe(title, () => {
it('visually looks correct', async () => {
await common.expectChartAtUrlToMatchScreenshot(`http://localhost:9001?id=${encodedGroup}--${encodedTitle}`);
const url = `http://localhost:9001?id=${encodedGroup}--${encodedTitle}`;
await common.expectChartAtUrlToMatchScreenshot(url);
});
});
});
Expand Down
86 changes: 86 additions & 0 deletions integration/tests/mixed-stories.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { common } from '../page_objects';
import { Fit } from '../../src/chart_types/xy_chart/utils/specs';

describe('Mixed series stories', () => {
describe('Fitting functions', () => {
describe('Area charts - no endValue', () => {
Object.values(Fit).forEach((fitType) => {
it(`should display correct fit for type - ${fitType}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts--fitting-functions-non-stacked-series&knob-seriesType=area&knob-dataset=all&knob-fitting function=${fitType}&knob-Curve=0&knob-End value=none&knob-Explicit valuve (using Fit.Explicit)=8`,
);
});
});
});

describe('Area charts - endValue set to 2', () => {
Object.values(Fit).forEach((fitType) => {
it(`should display correct fit for type - ${fitType}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts--fitting-functions-non-stacked-series&knob-seriesType=area&knob-dataset=all&knob-fitting function=${fitType}&knob-Curve=0&knob-End value=2&knob-Explicit valuve (using Fit.Explicit)=8`,
);
});
});
});

describe('Area charts - endValue set to "nearest"', () => {
Object.values(Fit).forEach((fitType) => {
it(`should display correct fit for type - ${fitType}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts--fitting-functions-non-stacked-series&knob-seriesType=area&knob-dataset=all&knob-fitting function=${fitType}&knob-Curve=0&knob-End value=nearest&knob-Explicit valuve (using Fit.Explicit)=8`,
);
});
});
});

describe('Area charts - with curved - endValue set to 2', () => {
Object.values(Fit).forEach((fitType) => {
it(`should display correct fit for type - ${fitType}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts--fitting-functions-non-stacked-series&knob-seriesType=area&knob-dataset=all&knob-fitting function=${fitType}&knob-Curve=1&knob-End value=2&knob-Explicit valuve (using Fit.Explicit)=8`,
);
});
});
});

describe('Area charts - Ordinal dataset - no endValue', () => {
Object.values(Fit).forEach((fitType) => {
it(`should display correct fit for type - ${fitType}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts--fitting-functions-non-stacked-series&knob-seriesType=area&knob-dataset=ordinal&knob-fitting function=${fitType}&knob-Curve=0&knob-End value=none&knob-Explicit valuve (using Fit.Explicit)=8`,
);
});
});
});

describe('Line charts - no endValue', () => {
Object.values(Fit).forEach((fitType) => {
it(`should display correct fit for type - ${fitType}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts--fitting-functions-non-stacked-series&knob-seriesType=line&knob-dataset=all&knob-fitting function=${fitType}&knob-Curve=0&knob-End value=none&knob-Explicit valuve (using Fit.Explicit)=8`,
);
});
});
});

describe('Line charts - endValue set to 2', () => {
Object.values(Fit).forEach((fitType) => {
it(`should display correct fit for type - ${fitType}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts--fitting-functions-non-stacked-series&knob-seriesType=line&knob-dataset=all&knob-fitting function=${fitType}&knob-Curve=0&knob-End value=2&knob-Explicit valuve (using Fit.Explicit)=8`,
);
});
});
});

describe('Line charts - with curve - endValue set to 2', () => {
Object.values(Fit).forEach((fitType) => {
it(`should display correct fit for type - ${fitType}`, async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts--fitting-functions-non-stacked-series&knob-seriesType=line&knob-dataset=all&knob-fitting function=${fitType}&knob-Curve=1&knob-End value=2&knob-Explicit valuve (using Fit.Explicit)=8`,
);
});
});
});
});
});
4 changes: 3 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ module.exports = {
roots: ['<rootDir>/src'],
preset: 'ts-jest',
testEnvironment: 'jest-environment-jsdom-fourteen',
setupFilesAfterEnv: ['<rootDir>/scripts/setup_enzyme.ts'],
setupFilesAfterEnv: ['jest-extended', '<rootDir>/scripts/setup_enzyme.ts', '<rootDir>/scripts/custom_matchers.ts'],
coveragePathIgnorePatterns: ['<rootDir>/src/mocks/', '<rootDir>/node_modules/'],
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markov00 I don't know why the coverage went down to 78% but it does not appear to be because of my code changes. Maybe this is causing some issues. Any thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the current coverage only cover the modules/files imported on each test. Adding the coveragePathIgnorePatterns maybe count also the missing modules without a test file importing them

clearMocks: true,
globals: {
'ts-jest': {
tsConfig: 'tsconfig.jest.json',
Expand Down
14 changes: 9 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@
"storybook:build": "rm -rf .out && build-storybook -c .storybook -o .out",
"lint": "NODE_ENV=production eslint --ext .tsx,.ts,.js .",
"lint:fix": "yarn lint --fix",
"test": "jest --verbose --config jest.config.js",
"pq": "pretty-quick",
"test": "jest --config jest.config.js",
"test:tz": "yarn test:tz-utc && yarn test:tz-ny && yarn test:tz-jp",
"test:tz-utc": "TZ=UTC jest --config=jest.tz.config.js",
"test:tz-ny": "TZ=America/New_York jest --config=jest.tz.config.js",
"test:tz-jp": "TZ=Asia/Tokyo jest --config=jest.tz.config.js",
"test:tz-utc": "TZ=UTC jest --verbose --config=jest.tz.config.js",
"test:tz-ny": "TZ=America/New_York jest --verbose --config=jest.tz.config.js",
"test:tz-jp": "TZ=Asia/Tokyo jest --verbose --config=jest.tz.config.js",
"watch": "yarn test --watch",
"semantic-release": "semantic-release",
"typecheck:src": "yarn build:ts --noEmit",
"typecheck:all": "tsc -p ./tsconfig.json --noEmit",
"playground": "cd .playground && webpack-dev-server",
"playground:ie": "cd .playground && webpack-dev-server --host=0.0.0.0 --disable-host-check --useLocalIp",
"jest:integration": "TZ=UTC JEST_PUPPETEER_CONFIG=integration/jest-puppeteer.config.js jest --rootDir=integration -c=integration/jest.config.js --runInBand"
"jest:integration": "TZ=UTC JEST_PUPPETEER_CONFIG=integration/jest-puppeteer.config.js jest --verbose --rootDir=integration -c=integration/jest.config.js --runInBand"
},
"files": [
"dist/**/*",
Expand Down Expand Up @@ -110,9 +110,12 @@
"husky": "^1.3.1",
"jest": "^24.9.0",
"jest-environment-jsdom-fourteen": "^0.1.0",
"jest-extended": "^0.11.2",
"jest-image-snapshot": "^2.11.0",
"jest-matcher-utils": "^24.9.0",
"jest-puppeteer": "^4.3.0",
"jest-puppeteer-docker": "^1.2.0",
"lodash": "^4.17.15",
"lorem-ipsum": "^2.0.3",
"luxon": "^1.11.3",
"moment-timezone": "^0.5.27",
Expand All @@ -132,6 +135,7 @@
"ts-jest": "^24.1.0",
"ts-loader": "^6.1.2",
"typescript": "^3.6.3",
"utility-types": "^3.8.0",
"webpack": "^4.29.5",
"webpack-cli": "^3.3.1",
"webpack-dev-server": "^3.3.1"
Expand Down
70 changes: 70 additions & 0 deletions scripts/custom_matchers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { matcherErrorMessage } from 'jest-matcher-utils';

// ensure this is parsed as a module.
export {};

/**
* Final Matcher type with `this` and `received` args removed from jest matcher function
*/
type MatcherParameters<T extends (this: any, received: any, ...args: any[]) => any> = T extends (
this: any,
received: any,
...args: infer P
) => any
? P
: never;

declare global {
namespace jest { // eslint-disable-line
interface Matchers<R> {
/**
* Expect array to be filled with value, and optionally length
*/
toEqualArrayOf(...args: MatcherParameters<typeof toEqualArrayOf>): R;
}
}
}

/**
* Expect array to be filled with value, and optionally length
*/
function toEqualArrayOf(this: jest.MatcherUtils, received: any[], value: any, length?: number) {
const matcherName = 'toEqualArrayOf';

if (!Array.isArray(received)) {
throw new Error(
matcherErrorMessage(
this.utils.matcherHint(matcherName),
`${this.utils.RECEIVED_COLOR('received')} value must be an array.`,
`Received type: ${typeof received}`,
),
);
}

const receivedPretty = this.utils.printReceived(received);
const elementCheck = received.every((v) => v === value);
const lengthCheck = length === undefined || received.length === length;

if (!lengthCheck) {
return {
pass: false,
message: () => `expected array length to be ${length} but got ${received.length}`,
};
}

if (!elementCheck) {
return {
pass: false,
message: () => `expected ${receivedPretty} to be an array of ${value}'s`,
};
}

return {
pass: true,
message: () => `expected ${receivedPretty} not to be an array of ${value}'s`,
};
}

expect.extend({
toEqualArrayOf,
});
Loading