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: prototype unit testing #712

Merged
merged 13 commits into from
Nov 14, 2020
1 change: 1 addition & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ jobs:
- run: yarn --frozen-lockfile
- run: yarn type-check
- run: yarn lint
- run: yarn test.clear
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is related to kulshekhar/ts-jest#1506 - I encountered an error when trying to run tests with jest a couple of times and clearing cache helped. We could get rid of for now and see if it breaks in CircleCI frequently, but it only adds about 2 seconds to a job.

Copy link
Contributor

Choose a reason for hiding this comment

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

It cannot affect CircleCI because it's a new container each time, with an empty cache - that's also why it takes 2s.
This justifies having the yarn command however, if it is useful for you locally.

The issue you linked to is closed however ("v26.1.4 is out, I will close this issue"), so you could try upgrading ts-jest (by upgrading jest itself) and that should fix your local problems as well.

- run: yarn test.prod
build:
docker:
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"release": "bash ./release.sh",
"release.docs": "yarn workspace @stoplight/elements release.docs",
"type-check": "yarn workspaces run type-check",
"test.clear": "yarn workspace @stoplight/elements test.clear && yarn workspace @stoplight/elements-utils test.clear",
"test.prod": "yarn workspace @stoplight/elements test.prod && yarn workspace @stoplight/elements-utils test.prod",
"prepublishOnly": "yarn build"
},
Expand Down
1 change: 1 addition & 0 deletions packages/elements-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"scripts": {
"build": "sl-scripts build",
"test": "jest",
"test.clear": "yarn test --clearCache",
"test.prod": "yarn test --coverage --maxWorkers=2",
"type-check": "tsc --noEmit"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/elements/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,7 @@ module.exports = {
setupFilesAfterEnv: ['./setupTests.ts'],
snapshotSerializers: ['enzyme-to-json/serializer'],
coveragePathIgnorePatterns: ['__tests__', '__fixtures__', '__stories__'],
moduleNameMapper: {
'^@stoplight/elements-utils$': '<rootDir>/../elements-utils/src',
},
};
6 changes: 5 additions & 1 deletion packages/elements/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"release.dryRun": "sl-scripts release --dry-run --debug",
"storybook": "start-storybook -p 9001",
"test": "jest",
"test.clear": "yarn test --clearCache",
"test.prod": "yarn test --coverage --maxWorkers=2",
"test.update": "yarn test --updateSnapshot",
"test.watch": "yarn test --watch",
Expand Down Expand Up @@ -86,8 +87,11 @@
"@stoplight/scripts": "8.2.3",
"@storybook/addon-knobs": "^5.3.19",
"@storybook/react": "^5.3.19",
"@testing-library/react": "^10.0.2",
"@testing-library/dom": "^7.26.5",
"@testing-library/jest-dom": "^5.11.5",
"@testing-library/react": "^11.1.1",
"@testing-library/react-hooks": "^3.4.2",
"@testing-library/user-event": "^12.2.0",
"@types/classnames": "^2.2.10",
"@types/enzyme": "3.x.x",
"@types/enzyme-adapter-react-16": "1.x.x",
Expand Down
34 changes: 34 additions & 0 deletions packages/elements/src/components/TryIt/__tests__/header.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import '@testing-library/jest-dom/extend-expect';

import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import * as React from 'react';

import { TryItHeader } from '../header';

test('Try It header renders correctly', () => {
render(<TryItHeader />);
const elem = screen.getByTestId('try-it-header');
expect(elem.textContent).toBe(' Try It Out');
expect(elem.innerHTML).toContain('data-icon="magic"');
expect(elem.innerHTML).toContain('data-icon="question-circle"');
Copy link
Contributor

Choose a reason for hiding this comment

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

This is implementation-dependent and unimportant

});

test('When hovered over question circle icon, Try It Out details are displayed', async () => {
render(<TryItHeader />);
const elem = screen.getByTestId('try-it-header');
const icon = elem.querySelector('[data-icon="question-circle"]') as Element;

userEvent.hover(icon);

expect(
await screen.findByText(
"Send HTTP requests to your API, or one of our mock servers, to see how it's going to respond. View the docs",
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Testing the copy is rarely a good idea either. The only thing it does is makes it more annoying to change it down the road, without providing any real safety.

Copy link
Contributor

Choose a reason for hiding this comment

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

But testing whether the popover works is a good one!

Now on how to select it? One really underrated and underutilized way of selecting things is by accessibility attributes (ARIA).

You may be familiar already that HTML5 includes many new "semantic" tags that behave the same as a regular div, but mean a slightly different thing. section, main, article, etc. Since HTML5, the official goal of HTML is to describe semantics, not to prescribe appearance or behavior. (We have CSS and JS for those, respectively.) We should utilize this semantic part of HTML more than we do right now, but that's another matter.

In addition to these, a more in-depth way to describe a web-page is using the HTML accessibility attributes: https://www.w3.org/TR/wai-aria-1.1/ & https://developer.mozilla.org/en-US/docs/Learn/Accessibility/WAI-ARIA_basics

While these are mainly designed to provide accessibility to websites - help screen readers and such - they implicitly let you describe the semantics of your site really well, the very semantics that you want to test. I tend to think of these as tools aiding testability and comprehension, and think of the accessibility improvement as a nice side-effect.

In this case using the tooltip role would be a perfect fit, then you could use the screen.findByRole selector to find the element and instead of asserting on the contents you could assert e.g. toBeVisible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing the copy is rarely a good idea either. The only thing it does is makes it more annoying to change it down the road, without providing any real safety.

I agree and will get rid of it

).toBeInTheDocument();

expect(await (await screen.findByText('here.')).closest('a')).toHaveAttribute(
'href',
'https://meta.stoplight.io/docs/studio/docs/Design-and-Modeling/05-request-maker.md',
);
});
2 changes: 1 addition & 1 deletion packages/elements/src/components/TryIt/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Popover, Position } from '@stoplight/ui-kit';
import * as React from 'react';

export const TryItHeader: React.FC = () => (
<h2 className="opacity-75 ml-1 mt-10 mb-8 font-medium flex items-center">
<h2 data-testid="try-it-header" className="opacity-75 ml-1 mt-10 mb-8 font-medium flex items-center">
<div className="flex-1">
<FontAwesomeIcon icon={faMagic} className="mr-4" /> Try It Out
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/elements/src/containers/API.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ const APIImpl = withRouter<APIProps>(function API(props) {

return (
<InlineRefResolverProvider document={document}>
<div className="APIComponent flex flex-row">
<div data-testid="APIComponent" className="APIComponent flex flex-row">
{layout === 'stacked' ? (
<StackedLayout uriMap={uriMap} tree={tree} />
) : (
Expand Down
Loading