Skip to content

Commit

Permalink
Add lint-test script (#3996)
Browse files Browse the repository at this point in the history
## Description

I have noticed that there's a `lint` and `lint-storybook` command in the
pre-push hook, but there is no `lint-test`. So I added one and fixed
some of the errors.

Some of the other errors are conflicting with my other pull request so
my next steps will be:

1. Merge everything I have open at the moment
2. Fix all the lint-test violations
3. Add lint-test to pre-push hook
4. Figure out how to do a typescript check in pre-push too

## Related Issue

Couldn't find one

## Motivation and Context

We want code style and type safety in the `test` folder too

## How Has This Been Tested?

npm run test

## Screenshots (if appropriate):

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist:

- [X] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have added tests to cover my changes.
- [ ] I have added a storybook story or extended an existing story to
show my changes
- [X] All new and existing tests passed.
  • Loading branch information
PavelVanecek committed Nov 26, 2023
1 parent 1379ec6 commit 68192fe
Show file tree
Hide file tree
Showing 12 changed files with 43 additions and 30 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"test-coverage": "vitest run --config vitest.config.ts --coverage",
"test-watch": "vitest --config vitest.config.ts",
"lint": "eslint \"./src/**/*.{ts,tsx}\"",
"lint-test": "eslint \"./test/**/*.{ts,tsx}\"",
"lint-storybook": "eslint \"./storybook/**/*.{ts,tsx}\"",
"autofix": "eslint \"./src/**/*.{ts,tsx}\" --fix",
"analyse": "cross-env NODE_ENV=analyse webpack ./src/index.ts -o umd/Recharts.js",
Expand Down
1 change: 1 addition & 0 deletions test/cartesian/CartesianAxis.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import { vi } from 'vitest';
import { Surface, CartesianAxis } from '../../src';

const CustomizeLabel = ({ x, y }: any) => (
Expand Down
1 change: 1 addition & 0 deletions test/cartesian/Scatter.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { fireEvent, render } from '@testing-library/react';
import { vi } from 'vitest';
import { Surface, Scatter } from '../../src';

describe('<Scatter />', () => {
Expand Down
52 changes: 28 additions & 24 deletions test/cartesian/YAxis.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import React from 'react';
import each from 'jest-each';
import { render } from '@testing-library/react';
import { describe, test, it, expect } from 'vitest';
import { Surface, AreaChart, Area, YAxis, BarChart, Bar } from '../../src';
import { AxisDomain } from '../../src/util/types';

describe('<YAxis />', () => {
const data = [
Expand Down Expand Up @@ -40,12 +41,14 @@ describe('<YAxis />', () => {
expect(ticks[1].getAttribute('y')).toBe('297.5');
});

each([
const casesThatShowTicks: [number, AxisDomain, string][] = [
// [ticksLength, domain, textContentOfTickElement]
[5, [0, 10000], 10000],
[4, [0, 'dataMax'], 9800],
[4, [0, 'dataMax - 100'], 9800],
]).it('Should render %s ticks when domain={%s}', (length, domain, textContent) => {
[5, [0, 10000], '10000'],
[4, [0, 'dataMax'], '9800'],
[4, [0, 'dataMax - 100'], '9800'],
];

test.each(casesThatShowTicks)('Should render %s ticks when domain={%s}', (length, domain, textContent) => {
render(
<AreaChart width={600} height={400} data={data}>
<YAxis type="number" stroke="#ff7300" domain={domain} />
Expand All @@ -72,20 +75,19 @@ describe('<YAxis />', () => {
expect(ticks[1].getAttribute('y')).toBe('297.5');
});

each([[[0, 'dataMax + 100']], [[0, 'dataMax - 100']], [['auto', 'auto']]]).it(
'Should render 0 ticks when domain={%s} and dataKey is "noExist" ',
domain => {
render(
<AreaChart width={600} height={400} data={data}>
<YAxis stroke="#ff7300" domain={domain} />
<Area dataKey="noExist" stroke="#ff7300" fill="#ff7300" />
</AreaChart>,
);
const ticks = document.querySelectorAll('text');

expect(ticks).toHaveLength(0);
},
);
const casesThatDoNotShowTicks: [AxisDomain][] = [[[0, 'dataMax + 100']], [[0, 'dataMax - 100']], [['auto', 'auto']]];

test.each(casesThatDoNotShowTicks)('Should render 0 ticks when domain={%s} and dataKey is "noExist" ', domain => {
render(
<AreaChart width={600} height={400} data={data}>
<YAxis stroke="#ff7300" domain={domain} />
<Area dataKey="noExist" stroke="#ff7300" fill="#ff7300" />
</AreaChart>,
);
const ticks = document.querySelectorAll('text');

expect(ticks).toHaveLength(0);
});

it('Render 4 ticks', () => {
render(
Expand Down Expand Up @@ -220,9 +222,11 @@ describe('<YAxis />', () => {
const barsFirstHidden = wrapperFirstHidden.container.querySelectorAll('recharts-bar-rectangle > path');
const barsSecondHidden = wrapperSecondHidden.container.querySelectorAll('recharts-bar-rectangle > path');

// spreading into single array to match indices, as barsBothShowing will get Rectangles from the first Bar, then the second
expect([...Array.from(barsSecondHidden), ...Array.from(barsFirstHidden)].every((bar, i) => {
return bar.getAttribute('height') === barsBothShowing[i].getAttribute('height')
})).toBe(true);
// spreading to match indices, as barsBothShowing will get Rectangles from the first Bar, then the second
expect(
[...Array.from(barsSecondHidden), ...Array.from(barsFirstHidden)].every((bar, i) => {
return bar.getAttribute('height') === barsBothShowing[i].getAttribute('height');
}),
).toBe(true);
});
});
1 change: 1 addition & 0 deletions test/cartesian/getTicks.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { vi } from 'vitest';
import { getTicks } from '../../src/cartesian/getTicks';
import { CartesianTickItem } from '../../src/util/types';
import { CartesianAxisProps } from '../../src';
Expand Down
1 change: 1 addition & 0 deletions test/chart/RadarChart.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { fireEvent, render } from '@testing-library/react';
import { vi } from 'vitest';
import { RadarChart, Radar, PolarGrid, PolarAngleAxis, PolarRadiusAxis } from '../../src';

describe('<RadarChart />', () => {
Expand Down
7 changes: 5 additions & 2 deletions test/component/Legend.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import { vi } from 'vitest';
import { Legend, LineChart, Line } from '../../src';

describe('<Legend />', () => {
Expand Down Expand Up @@ -53,7 +54,7 @@ describe('<Legend />', () => {
expect(container.querySelectorAll('.recharts-default-legend .recharts-legend-item line')).toHaveLength(2);
});

test('Does not render `strokeDasharray` (if not present) in Legend when iconType is set to something else than `plainline`', () => {
test('Does not render `strokeDasharray` (if not present) when iconType is not set to `plainline`', () => {
const { container } = render(
<LineChart width={600} height={300} data={data}>
<Legend iconType="line" />
Expand Down Expand Up @@ -105,7 +106,9 @@ describe('<Legend />', () => {
});

expect(consoleWarn).toHaveBeenCalledWith(
`The name property is also required when using a function for the dataKey of a chart's cartesian components. Ex: <Bar name="Name of my Data"/>`,
'The name property is also required when using ' +
"a function for the dataKey of a chart's cartesian components. " +
'Ex: <Bar name="Name of my Data"/>',
);
});
});
2 changes: 1 addition & 1 deletion test/component/Text.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render, screen } from '@testing-library/react';
import React from 'react';
import { Surface, Text } from '../../src';
import { vi } from 'vitest';
import { Surface, Text } from '../../src';

describe('<Text />', () => {
const mock = {
Expand Down
1 change: 1 addition & 0 deletions test/component/Tooltip.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { fireEvent, getByText, render } from '@testing-library/react';
import React from 'react';
import { vi } from 'vitest';

import {
Area,
Expand Down
1 change: 1 addition & 0 deletions test/util/CartesianUtils.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { scaleLinear, scaleBand } from 'victory-vendor/d3-scale';
import { vi } from 'vitest';
import {
ScaleHelper,
createLabeledScales,
Expand Down
4 changes: 1 addition & 3 deletions test/util/DomUtils.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { render, screen } from '@testing-library/react';
import React from 'react';
import { vi } from 'vitest';
import { getStringSize } from '../../src/util/DOMUtils';


/**
* getBoundingClientRect always returns 0 in jsdom, we can't test for actual returned string size
* Execution order matters
Expand Down Expand Up @@ -33,6 +33,4 @@ describe('DOMUtils', () => {
height: 17,
});
});


});
1 change: 1 addition & 0 deletions test/util/ReactUtils.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { render } from '@testing-library/react';
import React from 'react';
import { vi } from 'vitest';

import { Bar, Line, LineChart } from '../../src';
import {
Expand Down

0 comments on commit 68192fe

Please sign in to comment.