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(jest): create jest config package for unit testing #3678

Merged
merged 7 commits into from
Feb 8, 2023

Conversation

SychO9
Copy link
Member

@SychO9 SychO9 commented Nov 13, 2022

Changes proposed in this pull request:
This is part one of two PRs to introduce tests to our JavaScript codebase. This PR lays the foundation by introducing a flarum-jest-config package and the necessary changes to begin unit testing. An example core abbreviateNumber util test is added as well.

Reviewers should focus on:

  • Anything I might be missing about introducing Jest testing.

Part two is here: #3679

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).
  • Core developer confirmed locally this works as intended.
  • Tests have been added, or are not appropriate here.

Required changes:

  • Related documentation PR: (Remove if irrelevant)
  • Related core extension PRs: (Remove if irrelevant)

@SychO9 SychO9 requested a review from a team as a code owner November 13, 2022 10:04
@SychO9 SychO9 marked this pull request as draft November 13, 2022 10:05
@SychO9
Copy link
Member Author

SychO9 commented Nov 13, 2022

need to add the workflow

@SychO9 SychO9 removed the request for review from a team November 13, 2022 13:49
@SychO9 SychO9 force-pushed the sm/jest-unit-tests branch from 7783ea2 to f043035 Compare November 13, 2022 14:39
@SychO9 SychO9 marked this pull request as ready for review November 13, 2022 15:55
@SychO9 SychO9 self-assigned this Nov 15, 2022
@SychO9 SychO9 requested review from davwheat and imorland December 13, 2022 20:31
@luceos luceos self-requested a review December 13, 2022 20:31
Copy link
Member

@askvortsov1 askvortsov1 left a comment

Choose a reason for hiding this comment

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

Wonderful, thanks so much for making this work! I think the only uncertainty I have is whether it would be better to put test code next to the source code, either in the same folder or in a __tests__ subdir. That way, it's easier to keep track of source code having corresponding tests, although coverage should help with that too.

js-packages/jest-config/README.md Outdated Show resolved Hide resolved
@@ -72,33 +72,11 @@ module.exports = function (options = {}) {
{
// Matches .js, .jsx, .ts, .tsx
// See: https://regexr.com/5snjd
test: /\.(j|t)sx?$/,
test: /\.[jt]sx?$/,
Copy link
Member

Choose a reason for hiding this comment

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

A lot cleaner, yay!

Co-authored-by: Alexander Skvortsov <[email protected]>
@SychO9
Copy link
Member Author

SychO9 commented Jan 17, 2023

Wonderful, thanks so much for making this work! I think the only uncertainty I have is whether it would be better to put test code next to the source code, either in the same folder or in a tests subdir. That way, it's easier to keep track of source code having corresponding tests, although coverage should help with that too.

I don't have any strong opinions about this at the moment. I'm okay with it either way. We will probably find out as we write more tests in the future.

@askvortsov1
Copy link
Member

Let's keep as is for now then, LGTM!

Signed-off-by: Sami Mazouz <[email protected]>
@SychO9 SychO9 merged commit e7fc29a into main Feb 8, 2023
@SychO9 SychO9 deleted the sm/jest-unit-tests branch February 8, 2023 21:02
@SychO9 SychO9 added this to the 1.7 milestone Feb 8, 2023
@SychO9 SychO9 added the javascript Pull requests that update Javascript code label Feb 11, 2023
@luceos luceos mentioned this pull request Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code prio/high type/feature type/infrastructure
Projects
Status: completed
Development

Successfully merging this pull request may close these issues.

2 participants