Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

Restructure Provider component tests #746

Merged
merged 4 commits into from
Aug 2, 2019
Merged

Restructure Provider component tests #746

merged 4 commits into from
Aug 2, 2019

Conversation

vkrol
Copy link
Contributor

@vkrol vkrol commented Aug 2, 2019

Well, in that case, would you mind adding some tests for the Provider? If I see correctly, we have none. In theory, this change shouldn't break anything, but better to be sure than sorry. Perhaps even add those tests in a separate PR to see them passing and then merge here.

#739 (comment)

We have some tests in inject.test.js. I will move them in a separate file, review them, refactor them if necessary and probably write new ones.

#739 (comment)

danielkcz
danielkcz reviewed Aug 2, 2019
View reviewed changes
test/Provider.test.js Outdated
import { render } from "@testing-library/react"
import renderer, { act } from "react-test-renderer"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't follow why to use this one when line above you have RTL. Let's not mix two approaches in a single test file please :)

Copy link
Contributor Author

@vkrol vkrol Aug 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I don't follow why to use this one when line above you have RTL. Let's not mix two approaches in a single test file please :)

Sure. It is already done :) I will do this step by step. It was a non-final version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UPD: This is the final version of the PR.

@vkrol vkrol marked this pull request as ready for review August 2, 2019 20:03
@danielkcz
Copy link
Contributor

Looks great, thanks!

@danielkcz danielkcz merged commit 3e7bf8f into mobxjs:master Aug 2, 2019
@vkrol
Copy link
Contributor Author

vkrol commented Aug 2, 2019

@FredyC thank you!

@vkrol vkrol deleted the restructure-provider-component-tests branch August 2, 2019 21:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danielkcz danielkcz danielkcz left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vkrol @danielkcz