Incompatible with tests in spec/ written with JavaScript

[erichiggins]

Something about installing lineman-browserify causes issues with the lineman spec-ci command.

Steps to reproduce:

1. Create a project: lineman new project
2. Enter the directory: cd project/
3. Create a new simple test named: spec/foobar-spec.js¹
4. Run the tests to confirm: lineman spec-ci and expect # tests 2 # pass 2
5. Install lineman-browserify: npm install --save-dev lineman-browserify
6. Run tests again and see reduced tests: # tests 1 # pass 1

wat?

[1] Contents of spec/foobar-spec.js:

describe('foobar', function() {
  it('foobar test', function() {
    expect(true).toEqual(true);
  });
});

[searls]

Confirmed that this is Super Broken. I blame @davemo.

(Researching)

[searls]

Okay, I may have exaggerated by calling this assuredly "broken". What I will say is that the default vanilla files do you zero favors in informing how tests need to be structured. I had an example project someplace that had working tests (using require and all), but I can't find them.

Paging @jasonkarns in case he remembers.

[erichiggins]

Thanks for confirming -- I'd call it "broken enough" ;)

If you manage to find your example that allows me to work around it, that'd be much appreciated until this issue is closed.

[erichiggins]

Any updates?

[searls]

Hey Eric, I spent about an hour on this last week and I was pretty stumped by what was going on. I could get tests to work in some cases and not in others. I tried dramatically changing things so that tests would be concatenated traditionally, but then of course they'd lose their ability to invoke require.

I think this will take more thought than I have to give it. @jasonkarns or @davemo?

[davemo]

Ok, I dug into this for about an hour; a few notes:

1. I'm convinced that lineman is really optimized for working with concatenation as the base layer of assumption for how js is structured in both production code and tests.
2. Without changing our testing helpers to export via UMD or CJS, lineman-browserify as is only works kind of half baked and seems to support only the production side of things.
3. In general I'm not convinced Browserify makes sense in the ecosystem of vendor dependencies; we might be able to make this work if vendor deps stuck in vendor/js and CJS require statements were only relevant in our actual app/js files (as well as our tests which should require those bundles).
4. This is gnarly to debug :x

[erichiggins]

Thanks @davemo . Would you recommend that I rollback from using lineman-browserify in my project until this is resolved, or are there some workarounds?

[davemo]

Hey @erichiggins, my recommendation would be to avoid lineman-browserify for the time being. Unlike most lineman plugins, which have been extracted from our real-world needs using lineman for client work, lineman-browserify was mostly an experimental plugin that isn't in active use.

[djbender]

would be cool to have this in the docs somewhere!

[searls]

Please update the doc with a PR. We've really dropped the ball on making this plugin fit into all aspects of the app workflow IMO. Apologies :-(

On Thu, Sep 4, 2014 at 2:55 PM, Derek Bender notifications@github.com wrote:

would be cool to have this in the docs somewhere!

Reply to this email directly or view it on GitHub: https://github.com/linemanjs/lineman-browserify/issues/18#issuecomment-54527050

[djbender]

It's all good! This bit me in the butt today. Thankfully I found this issue.

On Thu, Sep 4, 2014 at 1:56 PM, Justin Searls notifications@github.com wrote:

Please update the doc with a PR. We've really dropped the ball on making this plugin fit into all aspects of the app workflow IMO. Apologies :-(

On Thu, Sep 4, 2014 at 2:55 PM, Derek Bender notifications@github.com wrote:

would be cool to have this in the docs somewhere!

Reply to this email directly or view it on GitHub:

https://github.com/linemanjs/lineman-browserify/issues/18#issuecomment-54527050

Reply to this email directly or view it on GitHub https://github.com/linemanjs/lineman-browserify/issues/18#issuecomment-54527254 .