jejacks0n / teaspoon

Teaspoon: Javascript test runner for Rails. Use Selenium, BrowserStack, or PhantomJS.
1.43k stars 243 forks source link

Teaspoon expands assets, causing applications with large amounts of javascript to load very slowly. #292

Closed bramski closed 9 years ago

bramski commented 9 years ago

Our application is a bit bloated, but once it's booted in development mode i don't have too much difficulty in getting things running rapidly.

However, I'm trying out teaspoon today and loading the initial page takes a huge amount of time. (that is the request to /teaspoon).

Are there some things I can configure to make it run faster? Thanks!

bramski commented 9 years ago

screen shot 2015-01-19 at 3 47 56 pm

bramski commented 9 years ago

Hmmm. I have managed to make this a little bit faster by adding a spec manifest and disabling file matching. But the load time is still a bit less than desireable.

bramski commented 9 years ago

screen shot 2015-01-19 at 3 56 56 pm

bramski commented 9 years ago

Actually not much better. What are you guys doing? my CPU is absolutely hammered.

jejacks0n commented 9 years ago

So I was willing to help, but your last comment indicates that you're probably not open to it being something other than teaspoon. I'll try anyway.

I structure my JavaScript dependencies, and recommend it in general. Eg. The spec file requires the implementation it's going to test, and the implementation requires its dependencies. I don't use one monolithic application.js to build everything. I'm guessing you're doing the later?

The reason for this is because I can run a single spec and load a minimum of JavaScript. If you use one monolithic manifest, rails has to re-generate that file even if only one file changed.


Jeremy Jackson

On Jan 19, 2015, at 5:19 PM, Bram Whillock notifications@github.com wrote:

Actually not much better. What are you guys doing? my CPU is absolutely hammered.

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

bramski commented 9 years ago

It doesn't take me 2 minutes to compile application.js.

bramski commented 9 years ago

And yes, that WOULD be nice. But it is a lot of work to eliminate the work of my terrible javascript predecessors on this application; but I do not feel that this is the fault of the application.js, because it takes 2 minutes even if zero files have changed.

bramski commented 9 years ago

And actually... application.js is fairly clean. It's all the vendor requirements and then it spirals down into separate files. Don't think thats the problem here. I'm profiling teaspoon right now to see whats up.

bramski commented 9 years ago

Hmmm. So I have identified the problem. There's 125 javascript spec files, and ..

<%= javascript_include_tag *@suite.spec_assets %>

turns out to be very slow. Rails (at least 3.2) is not well optimized for running javascript_include_tag on a large number of js and coffee files. It more or less forces rails to go through and do the entire recompilation each time as it's ending up doing the whole recompilation at each render of _boot.html.erb. WHY I am uncertain but I'm trying to dig into that.

bramski commented 9 years ago

Ahhhhh. SO here is the problem. Yes this is a curious difficulty with teaspoon. You are taking away work from sprockets and I'm not sure why. It has to do with this method:

def asset_tree(sources)
      sources.collect do |source|
        asset = @env.find_asset(source)
        Rails.logger.debug("finding asset #{source}")
        if asset && asset.respond_to?(:logical_path)
          asset.to_a.map { |a| asset_url(a) }
        else
          source unless source.blank?
        end
      end.flatten.compact.uniq
    end
bramski commented 9 years ago

When you find the asset and then expand it, you get back ALL of the files then included by that. and then you include them separately.... If the file is a manifest, would it not be effective to JUST include that file? In this case I have specified my helper file and attached a manifest as specified in the README.md:

    suite.use_framework :jasmine, "1.3.1"
    suite.matcher = nil
    suite.helper = "spec_helper"
bramski commented 9 years ago

BUT, this is causing teaspoon to expand my manifest and manually call javascript_include_tag for all files there contained and referenced by that. Some of which intelligently include each other when they are referenced alone which is causing a massive explosion of assets. Instead it should really just correctly reference the manifest I've specified and not expand my assets.

jejacks0n commented 9 years ago

Are you running it in dev mode? That's why teaspoon runs in dev mode by default -- you're convoluting how rails behaves with teaspoon, which is fair because it's complicated, but it sounds like you're running it in test/prod env.

bramski commented 9 years ago

I'm in pure development.

bramski commented 9 years ago

Well, now I have a local fork with expansion of assets disabled when the matcher is not present, but that's likely not a good solution for the future. But it will enable me to work with this for a while. But I would like to try and get a solution into teaspoon so I can eliminate my fork later.

This line is the specific area of difficulty... why is the source file being expanded as such? https://github.com/modeset/teaspoon/blob/master/lib/teaspoon/suite.rb#L59

bramski commented 9 years ago

This may be a curious difficulty with the nebulousness of the javascript in this project. But I'd like to seek some configuration or determination within teaspoon so I can use it on our project.

jejacks0n commented 9 years ago

it's being expanded because a line number and file name for an exception is pretty useless in one monolithic file, don't you think? What version of rails are you on, there's a known issue of slowness in sprockets that I submitted a PR for -- and has been fixed as of 3.2.13 or something like that -- http://weblog.rubyonrails.org/2013/2/27/Rails-3-2-13-rc1-has-been-released (search for Jeremy Jackson)

jejacks0n commented 9 years ago

I wish I had a good solution for you, but not knowing literally anything about your setup, you're free to add some profiling tools and whatnot -- you'll see that the majority of time is not within teaspoon, but rather, within sprockets/rails.

jejacks0n commented 9 years ago

Out of curiosity, how many files are you talking about here? 100+? If you look at a project like https://github.com/jejacks0n/mercury/tree/mercury2 you'll see that there's over 1k specs, and many many lines of code and files -- in coffeescript. This compiles and loads from scratch within 10 seconds on my 2012ish macbook pro, and additional changes to various files results in only those files being compiled, so subsequent loads take closer to 2-3 seconds. The specs run in less time than it takes to load usually. I'm still unsure why your files are being forced to concatenate as you say you're running in dev -- do you have some really strange asset configuration in dev? Like, they're never supposed to concat in dev, so something must be overridden that is making you experience it. Those are my final thoughts, let me know what you find.

bramski commented 9 years ago

Rails v3.2.21...

So in development mode, requesting a manifest file turns into rails including all the necessary files separately. It doesn't compile one monolothic file until production. So that's not really necessary in development. You will get proper exception lines from that.

bramski commented 9 years ago

And yes, it's 127 files.

bramski commented 9 years ago

Our application doesn't actually compile slowly at all. I have changed the title of this bug to be correct to what the problem is.

bramski commented 9 years ago

And there is a PR to fix it.

Emerson commented 9 years ago

Also experiencing super slow tests and would appreciate this being merged if possible :-)

jedschneider commented 9 years ago

@Emerson can you bundle from @bramski's github fork and tell us if it seems to improve your load time as well. Thanks!

Emerson commented 9 years ago

Hmm, still pretty slow - but I found the issue and it's specific to my application. My app is built in Ember and loads a ton of JS files (491 to be exact). In my development environment, I just serve up a single application.js file, which has obvious drawbacks, but provides me with the speed I need to develop efficiently.

Is there any way to tell Teaspoon to concat my assets into one file?

bramski commented 9 years ago

@Emerson if you disabled asset expansion then you're basically falling back onto how sprockets serves application.js in development or test mode, which is generally to add 491 includes. You'll have to tell rails to precompile (essentially run the app in production mode) to get around this.

Emerson commented 9 years ago

@bramski - ah, silly me - forgot to actually disable asset expansion. Is that just a config setting like:

config.expand_assets = false?

bramski commented 9 years ago

in teaspoon_env.rb:

config.suite do |suite|
    suite.expand_assets = false
Emerson commented 9 years ago

yup, just got it - but now every test is loading my entire merged JS - think I'm going to throw in the towel for now - this problem is specific to my app and the fact that I have so much JS.

mikepack commented 9 years ago

Closing this out and carrying the discussion over at #296