Plans for testing framework?

tofagerl commented 8 years ago

This is really interesting, but I can't help noticing that there is no testing framework set up as default. Is this on the agenda? Personally I find the testing part to be the worst part of the whole boilerplate madness, since you always end up needing plugins like jquery, and each plugin is responsible for documenting how it's configured. And if you're using something other than Karma, well... good luck!

Anywho, will you be adding a Karma config with some generic tests for the included default components to verify that it works out of the box?

I could do a PR, of course, but I'm so bad at configuring Karma it would probably end up failing everything...

dlmr commented 8 years ago

First of all, thanks for the offer!

It is partially by design that we do not have tests from the start just to show how Roc works. 🙂

We actually have a test plugin https://github.com/rocjs/roc-plugin-test-mocha-karma-webpack and just by installing it in you project you should get a new command named test that can be used to test your code. It will run using Karma, Mocha and PhantomJS by default and automatically give you access to expect.

npm install --save-dev roc-plugin-test-mocha-karma-webpack

After installing it just create a folder named tests in the root of the project and add some test files that end with .test.js using the default configuration as of now. You will also right now need to tell the test command where it can find the code that we would like to generate coverage for, this can be done through the CLI or using the configuration file. For now should be able to start the test command with this after creating the folder and installing the plugin.

roc test --tests-src-path app/

We are missing some documentation here and it should probably be included by default in the template to make it even easier to get started with tests as you say. If you want to create a PR that adds this plugin to the template with some default test that would be awesome!

We are working on making the documentation better and I will publish a article on Medium tomorrow that also walks through this process. If there is any issues I'm more than happy to provide a more detailed example.

tofagerl commented 8 years ago

I'll have a look tonight! Shouldn't be a big job :)

tofagerl commented 8 years ago

Turns out the big job is finding a testing workflow with expect.js which allows me to test dom elements. Or even access dom elements.

andreasrs commented 8 years ago

We should give that a little think. I'm not sure if a dom-test-lib should be included in the plugin. This is very dependent on the type of application you want to test and personal preferences. My vote goes to leave that outside of the plugin scope. What do you think @DLMR ?

dlmr commented 8 years ago

I agree in that we need to think about what makes sense to provide from the plugin. We want sane defaults and solve the most common use-cases and not create unnecessary abstractions that just creates another layer of complexity. We also need to think twice when making selection about things that can be highly personal, like testing frameworks to some extent. Some things might also be good to add to another plugin, if it is repetitive work that can be abstracted.

With that said, for some applications there will be a need to access DOM elements and it might be smart to provide a good pattern for that. For a React application, like this template, this is probably not as needed and things like Enzyme and React Test Utils can be used.

In the case of expect there is a extension that makes it easier, however not a way to select DOM elements. https://github.com/mjackson/expect-element However that can be done easily with just document.querySelector.

tofagerl commented 8 years ago

Unfortunately document.querySelector, document.getElementById and all of those are failing for me. They all return null. document itself returns this:

WARN LOG: 'document', <!DOCTYPE html><!--
This is the execution context.
Loaded within the iframe.
Reloaded before every execution run.
--><html><head>&#10;  <title></title>&#10;  <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">&#10;  <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1, user-scalable=no">&#10;</head>&#10;<body>&#10;  <!-- The scripts need to be in the body DOM element, as some test running frameworks need the body
       to have already been created so they can insert their magic into it. For example, if loaded
       before body, Angular Scenario test framework fails to find the body and crashes and burns in
       an epic manner. -->&#10;  <script type="text/javascript">&#10;    // sets window.__karma__ and overrides console and error handling&#10;    // Use window.opener if this was opened by someone else - in a new window&#10;    if (window.opener) {&#10;      window.opener.karma.setupContext(window);&#10;    } else {&#10;      window.parent.karma.setupContext(window);&#10;    }&#10;&#10;    // All served files with the latest timestamps&#10;    window.__karma__.files = {&#10;  '/base/node_modules/mocha/mocha.js': '850a41c1571b8b60a69a80e154f26c7d2c55fbbd',&#10;  '/base/node_modules/karma-mocha/lib/adapter.js': '9112a0a88c794e8094a758094ffc9093543c3edd',&#10;  '/base/node_modules/roc-plugin-test-mocha-karma-webpack/lib/karma/utils/entry.js': '4539c924dd6d9667e56dca77b5cc00aaed88ddc1'&#10;};&#10;&#10;  </script>&#10;  <!-- Dynamically replaced with <script> tags -->&#10;  <script type="text/javascript" src="/base/node_modules/mocha/mocha.js?850a41c1571b8b60a69a80e154f26c7d2c55fbbd"></script>&#10;<script type="text/javascript" src="/base/node_modules/karma-mocha/lib/adapter.js?9112a0a88c794e8094a758094ffc9093543c3edd"></script>&#10;<script type="text/javascript" src="/base/node_modules/roc-plugin-test-mocha-karma-webpack/lib/karma/utils/entry.js?4539c924dd6d9667e56dca77b5cc00aaed88ddc1"></script>&#10;  <script type="text/javascript">&#10;    window.__karma__.loaded();&#10;  </script>&#10;&#10;&#10;</body></html>

I'm sure there's a way around it, but I can't find it.

Full application.test.js:

import expect from 'expect';

describe('Basic tests that the application loads correctly', () => {
  it('#application exists in DOM', () => {
    let result = document.querySelector('div[id=application');
    console.warn('result', result);
    console.warn('document', document);
    expect(result.nodeName).toEqual('DIV');
  });
});

As for JSX testing, I assumed you would want that in a separate plugin, since that would make sense in templates for components as well as web-apps. Shallow rendering with either enzyme or expect-jsx.

tofagerl commented 8 years ago

I am currently learning about React testing, mostly because of this issue :) I'm leaning towards a solution which uses enzyme and expect-jsx, but if I ever get the time, I would like to create a fork of roc-plugin-test-mocha-karma-webpack which uses chai, chai-jsx and chai-react.

Enzyme seems like it has momentum, but if anyone would like to suggest react-addon-test-utils instead, feel free to wow me.

Otherwise, expect a PR in the near to far future...

andreasrs commented 8 years ago

Cool that you are learning about it :)

If you get something up and running that you like be sure to let us know.

tofagerl commented 8 years ago

Getting stuck on small things still. For example, Enzyme and webpack are at loggerheads, and while the solution seems to be to edit webpack.conf.js, but of course I still haven't figured out your hooking system.

So when I put this in my roc.config.js:

action: () => (rocObject) => {
    if (rocObject.hook = 'build-webpack') {
      const webpack = {
        externals: {
          'cheerio': 'window',
          'react-dom': 'window',
          'react/lib/ExecutionEnvironment': true,
          'react/lib/ReactContext': true
        },
      };
      return () => () => {
        Object.assign(rocObject.previousValue, webpack);
      };
    }
  }

I get this when I run roc test:

Found a local version of Roc, will use that over the global one.

Could not find the entry file for web at undefined
An error happened when trying to load local CLI, will use the global one.

Could not find the entry file for web at undefined

/Users/tom/dev/playground/node_modules/roc-package-webpack-web-dev/src/builder/index.js:35
            const makeAllPathsAbsolute = (input) => input.map((elem) => getAbsolutePath(elem));
                                                    ^
TypeError: Cannot read property 'map' of undefined
    at input.map (/Users/tom/dev/playground/node_modules/roc-package-webpack-web-dev/src/builder/index.js:35:53)
    at /Users/tom/dev/playground/node_modules/roc-package-webpack-web-dev/src/builder/index.js:37:31
    at /Users/tom/dev/playground/node_modules/roc/src/hooks/index.js:138:41
    at Array.map (native)
    at /Users/tom/dev/playground/node_modules/roc/src/hooks/index.js:119:30
    at Array.forEach (native)
    at runHookDirectly (/Users/tom/dev/playground/node_modules/roc/src/hooks/index.js:118:18)
    at /Users/tom/dev/playground/node_modules/roc/src/hooks/index.js:62:16
    at invokeHook (/Users/tom/dev/playground/node_modules/roc-plugin-test-mocha-karma-webpack/src/roc/util.js:21:12)
    at /Users/tom/dev/playground/node_modules/roc-plugin-test-mocha-karma-webpack/src/actions/test.js:10:28
    at /Users/tom/dev/playground/node_modules/roc/src/hooks/index.js:138:41
    at Array.map (native)
    at /Users/tom/dev/playground/node_modules/roc/src/hooks/index.js:119:30
    at Array.forEach (native)
    at runHookDirectly (/Users/tom/dev/playground/node_modules/roc/src/hooks/index.js:118:18)
    at /Users/tom/dev/playground/node_modules/roc/src/hooks/index.js:62:16

There are two problems here: the first is that I don't necessarily understand your hooking system, the second is that I don't understand why I should have to understand it.

I shouldn't have to use hooks, the hooks should be in the plugins with a simple if(config.webpack) then Object.assign(default.webpack, config.webpack)... Shouldn't it? This comes back to the discussion at https://github.com/rocjs/roc-plugin-test-mocha-karma-webpack/issues/5 and probably has to be solved at https://github.com/rocjs/roc, but instead of rewriting the hooking system, I would just like to get past integrating Enzyme for now.

Also, this could alternately be solved with a roc-plugin-webpack-enzyme which solved these problems for me, but I don't have the skills to make that one yet. Though I am learning...

Edit: Oh, background on enzyme/webpack: http://airbnb.io/enzyme/docs/guides/webpack.html

dlmr commented 8 years ago

There are two problems here: the first is that I don't necessarily understand your hooking system, the second is that I don't understand why I should have to understand it.

I agree with you to 100%! This is something that should "Just work™️". Unfortunately we are not where we want to be in terms of this and it's of the highest priority to both make this better and document everything around it.

I shouldn't have to use hooks, the hooks should be in the plugins with a simple if(config.webpack) then Object.assign(default.webpack, config.webpack)... Shouldn't it?

Yes, this should be managed at a higher level to simplify what needs to be done in applications.

About trying to getting it to work, try this:

const merge = require('roc').merge; 
action: () => (rocObject) => {
    if (rocObject.hook = 'build-webpack') {
      const webpack = {
        externals: {
          'cheerio': 'window',
          'react-dom': 'window',
          'react/lib/ExecutionEnvironment': true,
          'react/lib/ReactContext': true
        },
      };
      return () => () => {
        return merge(rocObject.previousValue, { buildConfig: webpack });
      };
    }
  }

tofagerl commented 8 years ago

Yep, that did it. Enzyme is still not working, but at least now the hooks are in the right place so I can debug it properly :) Thanks!

tofagerl commented 8 years ago

Sigh, that didn't last long. I'm not sure if this one is the "next" error, or if this is simply one that was hidden by the last one.... Or even if this is a roc error or just a webpack error...

  action: () => (rocObject) => {
    if (rocObject.hook === 'build-webpack') {
      const webpack = {
        externals: {
          'cheerio': 'window',
          'react-dom': 'window',
          'react/lib/ExecutionEnvironment': true,
          'react/lib/ReactContext': true
        },
      };
      return () => () => {
        return merge(rocObject.previousValue, { buildConfig: webpack });
      };
    }
  }

 tom  ~  dev  playground  roc test                                                                                                                                                                                                    1
Found a local version of Roc, will use that over the global one.

29 04 2016 16:32:50.506:WARN [preprocess]: Can not load "webpack"!
  TypeError: arguments[i].apply is not a function
    at Compiler.apply (/Users/tom/dev/playground/node_modules/tapable/lib/Tapable.js:164:16)
    at OptionsApply.WebpackOptionsApply.process (/Users/tom/dev/playground/node_modules/webpack/lib/WebpackOptionsApply.js:62:18)
    at webpack (/Users/tom/dev/playground/node_modules/webpack/lib/webpack.js:22:48)
    at new Plugin (/Users/tom/dev/playground/node_modules/karma-webpack/index.js:50:17)
    at invoke (/Users/tom/dev/playground/node_modules/di/lib/injector.js:75:15)
    at Array.instantiate (/Users/tom/dev/playground/node_modules/di/lib/injector.js:59:20)
    at get (/Users/tom/dev/playground/node_modules/di/lib/injector.js:48:43)
    at /Users/tom/dev/playground/node_modules/di/lib/injector.js:71:14
    at Array.map (native)
    at Array.invoke (/Users/tom/dev/playground/node_modules/di/lib/injector.js:70:31)
    at [object Object].get (/Users/tom/dev/playground/node_modules/di/lib/injector.js:48:43)
    at instantiatePreprocessor (/Users/tom/dev/playground/node_modules/karma/lib/preprocessor.js:52:20)
    at Array.forEach (native)
    at /Users/tom/dev/playground/node_modules/karma/lib/preprocessor.js:68:21
    at Array.forEach (native)
    at createPreprocessor (/Users/tom/dev/playground/node_modules/karma/lib/preprocessor.js:67:12)
29 04 2016 16:32:50.570:INFO [karma]: Karma v0.13.22 server started at http://localhost:9876/
29 04 2016 16:32:50.584:INFO [launcher]: Starting browser PhantomJS
29 04 2016 16:32:51.340:INFO [PhantomJS 2.1.1 (Mac OS X 0.0.0)]: Connected on socket /#cnB2uWZab_3xg1lXAAAA with id 21473846
PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
  ReferenceError: Can't find variable: require
  at /Users/tom/dev/playground/node_modules/roc-plugin-test-mocha-karma-webpack/lib/karma/utils/entry.js:10 <- ../../../src/karma/utils/entry.js:8:0

PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 0 of 0 ERROR (0.073 secs / 0 secs)

 tom  ~  dev  playground 

dlmr commented 8 years ago

Looks like externals is assumed to be an array here, not sure that this is the problem but try to change to that. Could you push what you have locally somewhere? I might be able to take a look at it this weekend.

 action: () => (rocObject) => {
    if (rocObject.hook === 'build-webpack') {
      const webpack = {
        externals: {
          'cheerio': 'window',
          'react-dom': 'window',
          'react/lib/ExecutionEnvironment': true,
          'react/lib/ReactContext': true
        },
      };
      return () => () => {
        return rocObject.previousValue.buildConfig.externals.push(webpack.externals)
      };
    }
  }

tofagerl commented 8 years ago

Hehe, that's basically all I have :)

I'm still trying to do a simple test on the normal react-web package. it'll be on https://github.com/tofagerl/playground in a few minutes.

Webpack should take an object as externals, but I can reformat this into an array... (Edit: Nope, same error)

dlmr commented 8 years ago

Yeah, it will take a object but I might think something else is using an array as the value here and that means that we are overwriting something here.

tofagerl commented 8 years ago

Oh, so you need to reformat that array into an object if the user passes an object... That sounds like a job... Because of course Webpack isn't the only piece of software that doesn't specify types like that...

dlmr commented 8 years ago

This will be easy to manage with a new way to manage "project level configuration" but we are not there yet. So yes, Roc should definitely handle this.

dlmr commented 8 years ago

This in your roc.config.js will work:

  action: () => (rocObject) => {
    if (rocObject.hook === 'build-webpack') {
      const externals = {
        'cheerio': 'window',
        'react-dom': 'window',
        'react/lib/ExecutionEnvironment': true,
        'react/lib/ReactContext': true
      };
      return () => () => {
        rocObject.previousValue.buildConfig.externals = externals;
        return rocObject.previousValue;
      };
    }
  }

tofagerl commented 8 years ago

Yep.

Should be able to figure out the rest now..

tofagerl commented 8 years ago

And no... That actually breaks roc build, which in turn breaks roc dev

dlmr commented 8 years ago

Something like this should work:

action: () => (rocObject) => {
    if (rocObject.hook === 'build-webpack' && rocObject.settings.build.mode === 'test') {
      const externals = {
        'cheerio': 'window',
        'react-dom': 'window',
        'react/lib/ExecutionEnvironment': true,
        'react/lib/ReactContext': true
      };
      return () => () => {
        rocObject.previousValue.buildConfig.externals = externals;
        return rocObject.previousValue;
      };
    }
  }

Edit: I was a bit quick here, this should be better.

tofagerl commented 8 years ago

Ended up with this:

 action: () => (rocObject) => {
    if (rocObject.hook === 'build-webpack') {
      const externals = {
        'cheerio': 'window',
        'react-dom': 'window',
        'react/lib/ExecutionEnvironment': true,
        'react/lib/ReactContext': true
      };
      return () => () => {
        rocObject.previousValue.buildConfig.externals = merge(rocObject.previousValue.buildConfig.externals, externals);
        return rocObject.previousValue;
      };
    }
  }

Seems to work with both.

dlmr commented 8 years ago

That will work but you will get those externals also in production and dev.

tofagerl commented 8 years ago

which I don't want... should I change the hook?

dlmr commented 8 years ago

Look at this https://github.com/rocjs/roc-template-web-app-react/issues/8#issuecomment-216273379

Added a check for the build mode.

tofagerl commented 8 years ago

Ah, that comment showed empty for me until I reloaded.

rocjs / roc-template-web-app-react

Plans for testing framework? #8