mipearson / webpack-rails

Integrate webpack with your Ruby on Rails application
MIT License
543 stars 82 forks source link

Integration tests todo, suggested approach #26

Open febeling opened 8 years ago

febeling commented 8 years ago

WRT to the todo about integration tests that is in the README: I guess the problem with integration tests is the exception that is raised because of the missing manifest?

I put this below in environments/test.rb and it solved the problem with my tests, which were rendering views.

config.webpack.dev_server.host = 'localhost'
config.webpack.dev_server.port = 3000
config.webpack.dev_server.enabled = false
config.webpack.output_dir = "public/webpack"
config.webpack.public_path = "webpack"
config.webpack.manifest_filename = "manifest.json"

Unless this has any drawbacks I don't see yet, it might help to put into the README for others bumping into that question. What do you think?

percyhanna commented 8 years ago

I had to use the same config so that our test suite would run properly. Without it, I would just get errors saying that the manifest was missing/couldn't load bundle.

mipearson commented 8 years ago

Interesting - the default config should "just work" for tests. I'll double-check what config we're using at Marketplacer for tests.

mipearson commented 8 years ago

The only change here from the defaults I can see is config.webpack.dev_server.enabled = false.

This is necessary if you're precompiling your entry points - which we do for CI, but not for developer testing, as we don't want to have to recompile for every single change.

mipearson commented 8 years ago

@febeling @percyhanna Can you please describe the process you're using to run your tests? I'm having trouble understanding this issue.

The docs in the README explain that if you're precompiling your assets in test, you'll need to set dev_server to false - so I think I'm missing something here.

mipearson commented 8 years ago

Crucially - are you running the dev server in the background while integration tests are running? Or are you precompiling your assets?

Or - are you talking about tests that are failing that do not touch webpack'd assets at all?

febeling commented 8 years ago

@mipearson - yes, you're right. This only works with the dev server running a the next, or pre-compiling. Sorry, I didn't really minimize this. It did help me in a pragmatic way, even though it won't be a satisfying setup for CI or when assets aren't under version control (as they probably shouldn't be).

To be clear, what it does is this: it configures the port to be 3000 instead of 3808, so that the built-in server serves the existing assets too. I have the same app in dev env running next to it, so the assets are usually there and up-to-day.

On reflection, that is probably too messy to recommend to anyone. What is a better approach?

davetron5000 commented 8 years ago

FWIW, I was able to get integration tests running with PhantomJS/Poltergeist by doing the following:

# config/environments/test.rb
  config.webpack.dev_server.enabled = !(ENV["CI"] == 'true')

Then, in spec/rails_helper.rb:

  config.before(:suite) do
    unless ENV["CI"] == 'true'
      @pid = fork do
        puts "Child process starting webpack-dev-server"
        exec("./node_modules/.bin/webpack-dev-server --config config/webpack.config.js --quiet")
      end
    end
  end

  config.after(:suite) do
    unless ENV["CI"] == 'true'
      puts "Killing webpack-dev-server"
      Process.kill("HUP",@pid)
      begin
        Timeout.timeout(2) do
          Process.wait(@pid,0)
        end
      rescue => Timeout::Error
        Process.kill(9,@pid)
      end
    end
  end

This starts the dev server just for running tests locally. I then arranged for my CI to do rails webpack:compile and set CI=true.

ujh commented 8 years ago

The underlying issue is that you either need to have webpack running or precompiled assets for tests to run even if they're just integration tests that won't ever load these files. Yes, I can of course either start the server or compile the assets before running the tests. But wouldn't it be cleaner (at least on a conceptual level) to have a special case for the test case that wouldn't even check the manifest? Maybe I'm missing a case that this would catch though. The README should at least mention that you need to either run the server or precompile the assets for the tests.

davetron5000 commented 8 years ago

FWIW, my solution does require webpack and/or the assets to exist because I'm running a browser and clicking around in my test, specifically to test Angular's behavior in browser.

percyhanna commented 8 years ago

@davetron5000 In our Travis config, we just execute the following before running our tests:

TARGET=test ./node_modules/.bin/webpack --config config/webpack.config.js

# run tests
davetron5000 commented 8 years ago

I do that, too, but don't want to do that for running a single test locally. My setup checks for the CI env var to know which is which.