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

karma to jest migration completed #4229

Merged
merged 3 commits into from Sep 1, 2023

Conversation

polbene95
Copy link

@polbene95 polbene95 commented Aug 31, 2023

Closes #4107

In this PR we are migrating from Karma to Jest.

Extra changes:

  • In day.jsx, month.jsx, & week.jsx PropTypes.instanceOf(Element) was changed for PropTypes.object because the first one doesn’t work on server side render making the test fail.

  • In day.jsx we changed shouldFocusDay && this.dayEl.current.focus({ preventScroll: true }); for shouldFocusDay && this.dayEl.current?.focus({ preventScroll: true }); because in some test this.dayEl.current was null making the test fail.

What's missing:

  • Scripts for test, test:ci & test:watch need to be updated.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

⚠️ This pull request was not sent to the PullRequest network because the title contains "draft:".

@martijnrusschen
Copy link
Member

Looking good overall, would be good to get CI scripts running so we can have the test suite running.

package.json Outdated Show resolved Hide resolved
@polbene95 polbene95 changed the title Draft: karma to jest migration completed karma to jest migration completed Sep 1, 2023
Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

✅ This pull request was sent to the PullRequest network.


@polbene95 you can click here to see the review status or cancel the code review job.

Copy link

@pullrequest pullrequest bot left a comment

Choose a reason for hiding this comment

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

PullRequest Breakdown

Reviewable lines of change

+ 2,088
- 2,337

96% JavaScript (tests)
3% JavaScript
1% JSON
<1% Other (tests)

Generated lines of change

+ 1,862
- 693

Type of change

Feature - These changes are adding a new feature or improvement to existing code.
1 Message
⚠️ Due to its size, this pull request will be reviewed by PullRequest staff before being sent to the reviewer network, and the team will reach out if there are any concerns. After this pull request is sent to the network, please be aware that it will likely have a longer turnaround time and require multiple passes from our reviewers.

Copy link
Contributor

@Zarthus Zarthus left a comment

Choose a reason for hiding this comment

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

Thanks so much for doing this!

I reviewed all the non-test files and it gets a big thumbs up from me.

I skimmed over the new tests and they look good to me as well.

@martijnrusschen martijnrusschen merged commit aec2bec into Hacker0x01:main Sep 1, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate test framework away from Karma
3 participants