Skip to content

Tests for fixed bug in unsubscribing consumers #8

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+616 −324
Open

Tests for fixed bug in unsubscribing consumers #8

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
@@ -41,7 +41,7 @@
"enzyme-to-json": "^3.2.2",
"flow-bin": "^0.60.1",
"husky": "^0.14.3",
"jest": "^21.2.1",
"jest": "^22.1.4",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for a moment I thought there was no test setup in this package (confused it with unstated repo), so I've started creating one and I've bumped jest version by doing so - can revert that change if you dont like it

"lint-staged": "^6.0.0",
"prettier": "^1.9.1",
"prop-types": "^15.6.0",
145 changes: 143 additions & 2 deletions src/__tests__/__snapshots__/createReactContext.test.js.snap
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,147 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`with provider - after click 1`] = `
exports[`unsubscribes correct consumer 1`] = `
<ThemeToggler>
<Provider
value="light"
>
<button
onClick={[Function]}
>
Toggle theme
</button>
<Title>
<Consumer>
<h1
style={
Object {
"color": "#000",
}
}
>
Hello World
</h1>
</Consumer>
</Title>
<Title>
<Consumer>
<h1
style={
Object {
"color": "#000",
}
}
>
Foo
</h1>
</Consumer>
</Title>
<Title>
<Consumer>
<h1
style={
Object {
"color": "#000",
}
}
>
Bar
</h1>
</Consumer>
</Title>
</Provider>
</ThemeToggler>
`;

exports[`unsubscribes correct consumer 2`] = `
<ThemeToggler>
<Provider
value="light"
>
<button
onClick={[Function]}
>
Toggle theme
</button>
<Title
key=".0"
>
<Consumer>
<h1
style={
Object {
"color": "#000",
}
}
>
Hello World
</h1>
</Consumer>
</Title>
<Title
key=".2"
>
<Consumer>
<h1
style={
Object {
"color": "#000",
}
}
>
Bar
</h1>
</Consumer>
</Title>
</Provider>
</ThemeToggler>
`;

exports[`unsubscribes correct consumer 3`] = `
<ThemeToggler>
<Provider
value="dark"
>
<button
onClick={[Function]}
>
Toggle theme
</button>
<Title
key=".0"
>
<Consumer>
<h1
style={
Object {
"color": "#fff",
}
}
>
Hello World
</h1>
</Consumer>
</Title>
<Title
key=".2"
>
<Consumer>
<h1
style={
Object {
"color": "#fff",
}
}
>
Bar
</h1>
</Consumer>
</Title>
</Provider>
</ThemeToggler>
`;

exports[`with provider: with provider - after click 1`] = `
<ThemeToggler>
<Provider
value="dark"
@@ -27,7 +168,7 @@ exports[`with provider - after click 1`] = `
</ThemeToggler>
`;

exports[`with provider - init 1`] = `
exports[`with provider: with provider - init 1`] = `
<ThemeToggler>
<Provider
value="light"
28 changes: 25 additions & 3 deletions src/__tests__/createReactContext.test.js
View file
Original file line number Diff line number Diff line change
@@ -7,15 +7,21 @@ import Adapter from 'enzyme-adapter-react-16';

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

const maybeHideChildren = (children, hiddenChildren) =>
hiddenChildren
? React.Children.toArray(children).filter((child, i) => !hiddenChildren.includes(i))
: children;


type Theme = 'light' | 'dark';
// Pass a default theme to ensure type correctness
const ThemeContext: Context<Theme> = createReactContext('light');

class ThemeToggler extends React.Component<
{ children: Node },
{ theme: Theme }
{ theme: Theme, hiddenChildren: ?number[] }
> {
state = { theme: 'light' };
state = { theme: 'light', hiddenChildren: null };
render() {
return (
// Pass the current context value to the Provider's `value` prop.
@@ -30,7 +36,7 @@ class ThemeToggler extends React.Component<
>
Toggle theme
</button>
{this.props.children}
{maybeHideChildren(this.props.children, this.state.hiddenChildren)}
</ThemeContext.Provider>
);
}
@@ -72,3 +78,19 @@ test('with provider', () => {
wrapper.find('button').simulate('click');
expect(wrapper).toMatchSnapshot('with provider - after click');
});

test('unsubscribes correct consumer', () => {
const wrapper = mount(
<ThemeToggler>
<Title>Hello World</Title>
<Title>Foo</Title>
<Title>Bar</Title>
</ThemeToggler>
);

expect(wrapper).toMatchSnapshot();
wrapper.setState({ hiddenChildren: [1] });
expect(wrapper).toMatchSnapshot();
wrapper.find('button').simulate('click');
expect(wrapper).toMatchSnapshot();
})
Copy link
Owner

Choose a reason for hiding this comment

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

It's not really clear that this tests unsubscribing the right consumer. Instead of modifying the existing component, could you create a new one that wraps two different <Title>'s and hides one of them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what is the idea behind proposed tweaked test - I mean I'm not sure in what way would you see it tweaked.

This test is testing that correct component got unsubscribed (or rather that others are still subscribed). Each <Title/> has different children, so we can see if those components got updated. I agree though that this might not be as clear as it could be - mainly because it's a snapshot test.

765 changes: 447 additions & 318 deletions yarn.lock
View file

Large diffs are not rendered by default.