AnomalyInnovations / serverless-bundle

Optimized packages for ES6 and TypeScript Node.js Lambda functions without any configuration.
https://serverless-stack.com/chapters/package-lambdas-with-serverless-bundle.html
MIT License
536 stars 157 forks source link

Allow testMatch to be configurable in jest #55

Open gndelia opened 4 years ago

gndelia commented 4 years ago

I want to be able to pass different testMatch https://jestjs.io/docs/en/configuration#testmatch-arraystring for Jest to run tests in specific folders

Currently, it runs tests in the entire codebase. My case is that we have a bunch of microservices, with some shared code in the parent level, such as dependencies. serverless-bundle is one of them (so it's not installed in every MS, although it is listed in the serverless' plugin list).

Although the rest seems to work (for example, webpack), here https://github.com/AnomalyInnovations/serverless-bundle/blob/master/scripts/config/createJestConfig.js#L23 the testMatch is hardcoded, ponting to the entire codebase. So when I run tests from one service it gets run in others. Allowing this to be configurable would allow me to run tests in a subfolder (which is only the microservice I'm interested in)

Thanks in advance

jayair commented 4 years ago

Can you share the directory structure of your project? And where you are running the test command from?

gndelia commented 4 years ago

Sure

Let's assume there are two serverless projects in our monorepo

/proy1 
  |- package.json // can run npm test from here, referring to parent `package.json`
  |- file1.js
  |- file2.js
  |- serverless.yml
/proy2
  |- package.json // can run npm test from here, referring to parent `package.json`
  |- file3.js
  |- file4.js
  |- serverless.yml
Readme.md
package.json // => here serverless-bundle is installed, in the root folder, as well as other plugins

each specific package.json has dependencies that just belong to that service. The root one (below Readme.md has all the shared dependencies.

From each service's package.json we just refer to it by npx --prefix ./../ test - and the root package.json is in care of running serverless-bundle test. However, that runs the tests in the entire codebase (both projects) rather than just in the one that called it.

If we were able to customize the testMatch through command line or even a file, we could filter out the proper folder in each service proy1|proy2.

We are following this custom approach because we read that lerna or yarn workspaces does not play well with serverless - given our solution is still smal, we are following this approach.

We also have considering switching to serverless-webpack and keep ourselves the configuration, but most of the team does not have experience with that, so using this package has worked well so far.

Furthermore, allowing to configure testMatch should allow us to configure coverage per project (as that configuration is already allowed I think)

pal commented 4 years ago

We have something similar and the same need. Our structure is as follows

/proj
  package.json
  /lib
    db.js
    db.test.js
  /services
    /service-a
      handler.js
      handler.test.js
      serverless.yml
    /service-b
      handler.js
      handler.test.js
      serverless.yml

If I run yarn test from /proj then I expect to run all tests, which is working great, but when I run from /proj/services/service-a I would like to just run that folders' handler.test.js.

I believe being able to override testMatch would allow this, right?

gndelia commented 4 years ago

yes, you would override testMatch in each service folder (service-a, service-b)

jayair commented 4 years ago

I see. This makes sense. Is the testMatch property set through the package.json?

gndelia commented 4 years ago

I think you can, just like any jest config (I am not 100% sure for the case of this package; I've only used configs from CLI call here)

the only "problem" I see is that for array configurations like this, here the code is extending the base configuration with whatever is passed

https://github.com/AnomalyInnovations/serverless-bundle/blob/master/scripts/config/createJestConfig.js#L58

but in this case, it would be necessary to override it, because, otherwise, we will be matching the entire codebase, even though we pass a testMatch with a subdirectory

jeff-kilbride commented 4 years ago

Piggybacking, here, but it's the same basic issue with jest configuration. I got the following error when trying to add the verbose option:

These options in your package.json Jest configuration are not currently supported by serverless-bundle:

  • verbose

If you wish to override other Jest options, consider using serverless-webpack directly instead.

Looking through the createJestConfig.js file referenced above, there doesn't seem to be a reason to exclude verbose from the list of supported options. Am I missing something?

Also, the error is a bit confusing. The serverless-webpack package doesn't look like it uses jest. It has mocha, chai, and sinon in it's devDependencies. How would using serverless-webpack directly allow me to override other jest options?

Thanks!

gndelia commented 4 years ago

because you would maintain your own jest version and configuration, I'd assume 🤔

jeff-kilbride commented 4 years ago

Ha! Yeah, I didn't think of that... 😄

This is a great package, so I don't want to come across as harsh. I completely understand why it was modeled on the CreateReactApp stuff. However, CRA is really restrictive in what it allows you to do. To make configuration tweaks, you either have to eject the project or use another npm package like react-app-rewired. Not sure serverless-bundle needs to be as strict. I can definitely see requests for different jest config options dribbling in over time...

EDIT Just an idea, but how about honoring the settings in a jest.config.js file, if found, like you do for eslint configs? Also, is it possible to pass command line options to jest? I'm trying to update failing snapshots, which would normally be handled with jest -u.

jayair commented 4 years ago

@gndelia The monorepo setup where serverless-bundle is at the root level is pretty common. It's some thing we use internally as well. So we should handle this well.

Just looking into it we need to set the rootDir https://github.com/AnomalyInnovations/serverless-bundle/blob/master/scripts/config/createJestConfig.js#L32 by setting the appPath https://github.com/AnomalyInnovations/serverless-bundle/blob/master/scripts/config/paths.js#L35 correctly. That should do it right? We wouldn't need to set the testMatch then?

I think passing in a CLI option makes the most sense to me here? Then you could add that to the test command for your microservice. It would be great if we can do this automatically though. But I'll need to play around and see if something can be done.

@jeff-kilbride Maybe open a separate issue for the other Jest options? And we can figure out how to set that?

jayair commented 4 years ago

@gndelia @pal I looked into it a bit more and I got this to work. Let's take your structure:

/proy1 
  |- package.json // can run npm test from here, referring to parent `package.json`
  |- file1.js
  |- file2.js
  |- serverless.yml
/proy2
  |- package.json // can run npm test from here, referring to parent `package.json`
  |- file3.js
  |- file4.js
  |- serverless.yml
package.json // => here serverless-bundle is installed, in the root folder, as well as other plugins

In proy1/package.json use this as your test command:

  "scripts": {
    "test": "npm --prefix ./../ test proy1"
  },

This will only run the tests inside proy1/.

Let me know if that works for you.

gndelia commented 4 years ago

yes, that seemed to do the trick.

Thank you !!

gndelia commented 4 years ago

It's me again @jayair

I'm now, given this setup explained, trying to read coverage only from the MS where I've run the tests. I see here https://github.com/AnomalyInnovations/serverless-bundle/blob/master/scripts/config/createJestConfig.js#L12 that collectCoverageFrom is supported, but it will gracefully extend whatever I passed to the default that already covers all files. Perhaps should be overriden instead?

jayair commented 4 years ago

@gndelia Can you post the steps to recreate this?

gndelia commented 4 years ago

Sure it's the same scenario as we discussed before https://github.com/AnomalyInnovations/serverless-bundle/issues/55#issuecomment-590087775

when I run

"scripts": {
    "test": "npm --prefix ./../ test proy1"
  },

it runs the tests from that proy1, successfully.

Now, when I add the coverage option, it collects coverage from the entire solution. According to the code, I can customize jest with a jest option in package.json and I could specify from there a specific path to collect coverage from. However, the code is only picking up that jest option from the root package.json but not from the proy1/package.json.

I need to configure this in every proy/package.json and not in the root one because the path will change given the project (essentially, it would be something like proy1/**/**{!.test.}.js - I don't need anything fancier than that

(btw, I think current default collectCoverageFrom option should exclude files that end with .test.js - I think those are currently counted)

jayair commented 4 years ago

@gndelia Got it. Yeah I'll need to investigate this further.

gndelia commented 4 years ago

Let me know if I can help in any way!

jayair commented 4 years ago

@gndelia Thanks! My hunch is that it won't be possible to pass it through the package.json. So we might have to pass it as a command line option instead. We'll need to investigate a way to do that.

pal commented 4 years ago

Thanks for that idea @gndelia, it won't work in my case however, since I'm only using a single package.json in my root dir.

For now, I'm able to use a prefix on test names and run tests using yarn test -t TESTPREFIX, or npm test -t FOLDERNAME. 🤷‍♂

jayair commented 4 years ago

We could use some help looking into generating the coverage reports for monorepo setups!