Plugin requires folder src to be created in root

[tofagerl]

As said in topic, roc test fails with the following error if folder src is not in the project root.
If this is a feature, not a bug, shouldn't the entire App directory be in the src folder?

Error: Cannot find module "/Users/tom/dev/testing/src"
  at /Users/tom/dev/testing/node_modules/roc-plugin-test-mocha-karma-webpack/lib/karma/utils/entry.js:60 <- webpack:///~/roc-plugin-test-mocha-karma-webpack/lib/karma/utils/entry.js:14:0

https://asciinema.org/a/7pzre2zjrpp9bum0tj68d8m19
(I finally got a reason to use Asciinema!)

[dlmr]

As I said in rocjs/roc-template-web-app-react#8 (comment) you will here need to run the test command with an option that tells it where it can find the code to generate coverage for.

roc test --tests-src-path app/

This is because the template uses another pattern for where it hosts its src files, app/ over src/ that the test plugins expects by default.

I guess the question here is what the optimal action is:

• Update the roc.config.js in the template that informs the test plugin where the src files are located.

// …
test: {
  src: {
    path: 'app'
  }
}
// …

• Change the template to not use app/ for the src files, as you suggests.
• Change the default in the plugin to match the template. (Probably not what we want here.)

I'm leaning against nr. 1 for now. We should however manage the case when the src folder can't be found in a better way though not allowing it to end up as an error. Would probably be smart to check if the specified folder can be found, src/, and if not give a warning to the user and a possible solution to the problem.

[tofagerl]

"Cannot find source folder 'src'. Please use roc test --tests-src-path app/ to specify where your sources are kept." for instance.
Personally I prefer the convention of using /src/App, so that I can have a separate /src/lib, /src/helpers and so on, but that's all a matter of style over substance...

In any case, how about changing the default roc test script to use /App as default?

[dlmr]

Yeah something like that. We will however not be able to easily detect where the real source files are located.

You mean to change the configuration as in the first alternative above? I think that would be a good start. It's probably however more logical to change the template to use src/ over app/ in the future since that is more generic and will work if we are working with something that is not an application, like a component.

To keep in mind in regards to the template is that we should aim to keep it as minimal as possible and really let the community create alternative templates that work for their specific use case since it's so easy to create and use a new ones.

[tofagerl]

OK. Having slept on it, and then gone to the dentist on it, I suggest simply renaming app to src. /src/components, /src/reducers and so on.

Also, where should I place karma.conf.js for default tests? The only one I can find after installing the roc-plugin-test-mocha-karma-webpack dependency is the ones in the actual package itself (node_modules/roc-plugin-test-mocha-karma-webpack/lib/karma), which is fine, but it doesn't seem to look for any user-configurable settings file.
I assumed /karma.conf.js would work, but it doesn't. Is the same true for webpack?
Is it meant to be in roc.config.js like NWB?

[dlmr]

I agree, we should update the template to use src/ over app/ to make it more conventional and additionally make it work with the test plugin with minimal configuration, the goal with Roc in general. You want to make a PR that makes this change? 🙂

Short answer. Yes through the roc.config.js file.

Long answer. There is currently one way to do it using the roc.config.js file but we would like to make this more easy to use for projects, maybe to work more in line with you were expecting. We are lacking some documentation in this area but I will explain how it works now below.

Current way
Roc uses a concept of hooks that allows projects and extensions (packages & plugins) to extend other extensions. To modify or add things to the Karma configuration, or the Webpack configuration, you will need to use one of these hooks. The one that needs to be used is this one. You can read more about how the internals of Roc works here but let me cut to the chase and present how this can be done today.

// Inside the roc.config.js
module.exports = {
  // ...
  action: () => (rocObject) => {
    const hook = rocObject.hook;
    const previousValue = rocObject.previousValue;

    if (hook === 'build-karma-config') {
      return () => () =>
        Object.assign(previousValue, {
          // A karma configuration object that will
          // overwrite what the extensions has defined
          // You would also be able to mutate the previousValue
          // and return that directly.
        });
    }
  }
}

This type of interface is not going away but we are looking into make it simpler to work with, for instance will the first function in the chain probably be removed soon and maybe some abstraction is desirable as has been provided for extensions.

The future
We would very much like to support what you initially tested out or some variation of it, just being able to define a local karma.conf.js or webpack.config.js that will automatically be merged with the default or used to override whatever the extensions has defined can be nice. We already know how to do this from a technical perspective but the question is what makes the most sense. What do you think would be a nice interface? Is the nwb way the best pattern or can something better be implemented? Just to note, implementing it in the same way as nwb would be a quite trivial task using the current core of Roc.

[tofagerl]

There's a PR coming from me either way, if I can get the testing to work. Basically I'll do it in two parts (so two PRs). One tests for some pretty basic things - things that don't require anything other than the roc-test plugin with the long name. I can do a quick PR first with the app->src change, I just need to do some grepping to see what else needs to be changed so the PR can be merged with no carnage.

Then I've planned some tests using karma-jquery-expect, but you might not want to merge that because it obviously adds some dependencies. I'll still make it a PR and then there can be a discussion on whether to add it. So, yeah, three PRs.
By the way, Chai rocks. Just mentioning it...

As for configuration style, there is definitely something to be said for both keeping it in the same file for ease of use, and separation of concerns for reuse - not that you can't reuse parts from a huge config file of course.

How about making it optional like with gulp and webpack? Default is to have a karma: object in roc.config.js, but if you simply do a karma: require('karma.conf.js') then obviously you know what you're doing and Roc should just roll with it? Opinionated is great, but if you don't have to eliminate choices while still keeping things relatively clean, that's something I would appreciate.

[dlmr]

Sounds awesome, looking forward to it!

Just a heads up, this is the current default expect by mjackson, not this expect by Automattic. karma-jquery-expect seems to be coupled with the wrong one here.

Thanks for your input, will work more with this going forward.

[tofagerl]

This is what I hate about the microlibrary model. We need a microlib to connect microlib A to microlib B. Not that these are really small libraries, but you know...

[dlmr]

I will close this issue since the direct problem has been resolved through rocjs/roc-template-web-app-react#10. We can continue the discussion here or in another issue.