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.

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

Jump to bottom

Issue 173 remove enzyme tests #177

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Issue 173 remove enzyme tests #177

wants to merge 3 commits into from

Conversation

karlrez
Copy link
Contributor

@karlrez karlrez commented Aug 17, 2022

Resolves #173

Hey @gsambrotta, just wanted to post an update as its been a couple weeks, so I figured it would be a good idea to post a draft of the changes so far.

Currently two test files still include enzyme: TabsMenu.test.tsx and _app.test.tsx

I am stuck on these ones because these tests are testing component props, where React Test Library is more designed for testing DOM elements. It should be possible with a solution similiar to this . So I am still working on this! But if you have any ideas or want other changes let me know.

@karlrez
Copy link
Contributor Author

karlrez commented Sep 28, 2022 •
edited
Loading

Related Issue

Resolves #173

Description

Removed enzyme from tests, refactored tests to use React Test Library

Impacted Areas in Application

All test files using enzyme.

Steps to Test or Reproduce

Related PRs

PR Checklist:

  • My code follows the style guidelines of this project and I have double check my own code
  • I have made corresponding changes to the documentation, if any
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have run npm run test and be sure all test pass
  • I have run npm run build:dev and be sure no error blocks the build
  • I have test locally that everything run as expected
  • I have properly fill in the PR template
  • I have ask for reviews

Hi @gsambrotta, I've now got the enzyme tests changed over to React Test Library.

I only removed one test in TabsMenu.test.tsx where it was checking the child component props. The approach I would have done to rewrite the test to check the DOM elements was already done in the next test case, so I figured we are covered there.

I did try remove the enzyme library from the project and came across LayoutTest.tsx which threw an error while running build. I got stuck with the first test of just rendering out the component. Based on the comments in the file, I think you or someone else may have a better understanding of what the issue is here? That should be the last file to update, and then we should be able to remove the enzyme library from the project.

Let me know if you have any suggestions, I don't mind making further updates on this.

@karlrez karlrez marked this pull request as ready for review September 28, 2022 15:29
@karlrez karlrez requested a review from a team as a code owner September 28, 2022 15:29
@karlrez karlrez requested review from gsambrotta and removed request for a team September 28, 2022 15:29
@gsambrotta
Copy link
Collaborator

Hey @karlrez thank you very much for your work. I am sorry i did not replay to your first post, I thought i did, it should have slip it.
I will review your code soon. thank you for your time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsambrotta gsambrotta Awaiting requested review from gsambrotta gsambrotta is a code owner automatically assigned from ElliotForWater/elliotforwater-core-devs

At least 1 approving review is required to merge this pull request.

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

Successfully merging this pull request may close these issues.

Remove Enzyme from tests
2 participants
@karlrez @gsambrotta