Coverage does not show up with expand_assets set to false

[andresbravog]

opening a new one referencing #357 as is happening again to me.

[mikepack]

This test doesn't suffice? https://github.com/modeset/teaspoon/blob/master/spec/teaspoon/suite_spec.rb#L111-L116

[andresbravog]

I guess not.

Placing this in suite:

    def asset_url(asset)
      params = []
      params << "body=1" if config.expand_assets
      puts '>> Loading ' + asset.pathname.to_s.split('myreponame').last
      params << "instrument=1" if instrument_file?(asset.pathname.to_s)
      url = asset.logical_path
      url += "?#{params.join("&")}" if params.any?
      puts '>> |===> with url:  ' + url.split('myreponame').last
      url
    end

gives the output

>> Loading /spec/javascripts/spec_helper.coffee
>> |===> with url:  spec_helper.js?instrument=1
>> Loading /spec/javascripts/backbone/apps/profiles/edit/view/header_spec.coffee
>> |===> with url:  backbone/apps/profiles/edit/view/header_spec.js
>> Loading /spec/javascripts/backbone/apps/profiles/show/view/basic_custom_field_header_spec.coffee
>> |===> with url:  backbone/apps/profiles/show/view/basic_custom_field_header_spec.js
>> Loading /spec/javascripts/backbone/apps/profiles/show/view/contact_email_spec.coffee
>> |===> with url:  backbone/apps/profiles/show/view/contact_email_spec.js
>> Loading /spec/javascripts/backbone/apps/profiles/show/view/contact_link_spec.coffee
>> |===> with url:  backbone/apps/profiles/show/view/contact_link_spec.js
>> Loading /spec/javascripts/backbone/apps/profiles/show/view/contact_phone_spec.coffee
>> |===> with url:  backbone/apps/profiles/show/view/contact_phone_spec.js
>> Loading /spec/javascripts/backbone/apps/profiles/show/view/current_experience_spec.coffee
>> |===> with url:  backbone/apps/profiles/show/view/current_experience_spec.js
>> Loading /spec/javascripts/backbone/apps/profiles/show/view/experience_custom_fields_header_spec.coffee
>> |===> with url:  backbone/apps/profiles/show/view/experience_custom_fields_header_spec.js
>> Loading /spec/javascripts/backbone/apps/profiles/show/view/layout_spec.coffee
>> |===> with url:  backbone/apps/profiles/show/view/layout_spec.js
>> Loading /spec/javascripts/backbone/apps/settings/settings_spec.coffee
>> |===> with url:  backbone/apps/settings/settings_spec.js
>> Loading /spec/javascripts/backbone/entities/filters/filters_spec.coffee
>> |===> with url:  backbone/entities/filters/filters_spec.js
.........................

Finished in 0.09300 seconds
25 examples, 0 failures
Unable to generate cobertura coverage report.

so the instrumentation flag is required but it's not happening. Going back to config.expand_assests = true works nice.

I'll keep investigating

[andresbravog]

Finally it was related to default coverage ignore regex that includes %r{(+.)_helper.} ignoring the spec_helper file.

Just created a PR solving this :)

[jejacks0n]

I don't know if I'm a fan of the spec helper being instrumented by default. Can you give me some examples of the code in it that you want to make sure are covered? I've never put enough code in there that had logic that I could change without breaking specs.

---

Jeremy Jackson

On Aug 18, 2015, at 12:17 AM, Andrés Bravo notifications@github.com wrote:

Finally it was related to default coverage ignore regex that includes %r{(+.)_helper.} ignoring the spec_helper file.

Just created a PR solving this :)

— Reply to this email directly or view it on GitHub.

[andresbravog]

@jejacks0n Well the default spec_helper stands for:

# You can require your own javascript files here. By default this will include everything in application, however you
# may get better load performance if you require the specific files that are being used in the spec that tests them.
#= require application

So by default all my application code is loaded there, that's why I want it to be instrumented.

[jejacks0n]

But see, that's just a lazy way of managing dependencies and one that impacts performance when focus running individual spec files. We figured out what you were after and you have a solution to it that you can configure. Can we just add a note to the readme or the wiki, along with your results in the open issue for others to learn from? This PR forces people who manage dependency trees (requiring their implementation files in their spec files) to have to always add the spec helper to their ignore list. Maybe there's a change to the config to make removing them easier, but I don't think this is the one to merge.

---

Jeremy Jackson

On Aug 18, 2015, at 3:03 AM, Andrés Bravo notifications@github.com wrote:

@jejacks0n Well the default spec_helper stands for:

You can require your own javascript files here. By default this will include everything in application, however you

may get better load performance if you require the specific files that are being used in the spec that tests them.

= require application

So by default all my application code is loaded there, that's why I want it to be instrumented.

— Reply to this email directly or view it on GitHub.

[andresbravog]

I understand your concerns, thing is:

If you want the coverage to work with the default values of this gem this is one way to go. Will appreciate any other idea, but IMO the the gem features should work with default values.

The current Readme does not work for coverage (at least if you are not using require.js). It took me several hours of gem debugging to understand that what was failing is the instrumentation and a little bit more to find the set of configuration options that makes them work. That knowledge should be more accessible as you said.

[jejacks0n]

I'm not promoting requirejs, I'm promoting having an implementation file, and a spec file that requires that using #= require implementation.coffee. The spec helper should be used for dependencies and only dependencies -- though the generator includes the #= require application as a default to help people get started with testing.

I believe the default does work with coverage -- you have a modified version (maybe with config.expand_assets=false?, and if you restored it, I'm pretty sure it would work. If this is not the case please let me know.