Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallelising builds: Update info about knapsack gem #99

Merged
merged 4 commits into from
May 19, 2017

Conversation

ArturT
Copy link
Contributor

@ArturT ArturT commented Mar 26, 2017

I added a few more tests runners in knapsack gem.

I also worked with a few companies with large projects. They use Buildkite and knapsack_pro gem. Often people ask me how to run a project with knapsack_pro and what CI provider I recommend and my first choice is Buildkite because it can be really fast comparing to other solutions.

I prepared article with a simple introduction to Buildkite:
http://docs.knapsackpro.com/2017/auto-balancing-7-hours-tests-between-100-parallel-jobs-on-ci-buildkite-example
and examples of configured Ruby/Rails project + knapack_pro with and without Docker:

Here is related PR for list of pipelines:
buildkite/example-pipelines#4

@toolmantim
Copy link
Contributor

Thanks @ArturT! I've done some testing before we merge this in, and I have some problems getting capybara-screenshot behaving. I'll resolve it with you on email/Slack and then we can get this in.

@ArturT
Copy link
Contributor Author

ArturT commented Mar 29, 2017

One of knapsack_pro users found the problem with capybara-screenshot. I added info how to solve it here:
https://github.com/KnapsackPro/knapsack_pro-ruby#how-to-use-queue-mode

If you will encounter a problem with stack level too deep then you may want to ensure you load your dependencies only once in spec/rails_helper.rb or spec/spec_helper.rb. The Queue Mode may load multiple times the rails_helper.rb hence the problem. For instance, the problem occurs for capybara-screenshot gem. Here is the example how you should load the gem.

unless ENV['KNAPSACK_PRO_RSPEC_DEPENDENCIES_LOADED']
  ENV['KNAPSACK_PRO_RSPEC_DEPENDENCIES_LOADED'] = 'true'
  require 'capybara-screenshot/rspec'
end

@ArturT
Copy link
Contributor Author

ArturT commented Mar 29, 2017

@toolmantim
Copy link
Contributor

@ArturT ah, I tried that but couldn't get it working. I've made some more adjustments to make sure the env check happens between the require and the bind.

I'm running some more tests now… and I'd love to fix the underlying cause inside capybara-screenshot.

Do you know what part of knapsack might be causing that, so I can dig some more?

@ArturT
Copy link
Contributor Author

ArturT commented Mar 29, 2017

I was looking at the capybara-screenshot the other day but without luck what was the root problem.

Here is how I load RSpec
https://github.com/KnapsackPro/knapsack_pro-ruby/blob/master/lib/knapsack_pro/runners/queue/rspec_runner.rb#L42
to run subset of the work queue. Then I clear rspec world and again run subset of work queue.

Probably the problem is with RSpec that may load rails_helper.rb/spec_helper.rb again. require 'capybara-screenshot/rspec' should load once but it seems not or RSpec is caching something.

The workaround with ENV['KNAPSACK_PRO_RSPEC_DEPENDENCIES_LOADED'] seems to be good enough and can be reused if another gem will cause a similar problem.

@toolmantim
Copy link
Contributor

I sent you more details over the Buildkite Slack. Any chance you could jump on? Or support email better?

@ArturT
Copy link
Contributor Author

ArturT commented Mar 29, 2017

@toolmantim You can send me details on my email. I will have more time to dig it later this evening (I'm in CEST).

@toolmantim
Copy link
Contributor

@ArturT I have a recreated the smallest possible example to show the error: https://gist.github.com/toolmantim/2a1d73fd2f8cca15f38255995af035a2 I hope that helps to figure out what's going on!

@ArturT
Copy link
Contributor Author

ArturT commented Mar 31, 2017

I found root problem.

https://github.com/mattheworiordan/capybara-screenshot/blob/master/lib/capybara-screenshot/rspec.rb#L91

Here is before(:suite) block. It is expected to run only once before whole test suite.
But in Knapsack Pro Queue Mode then RSpec runner is run multiple times for each subset of test files from work queue. It means the before(:suite) will be executed for each subset of test files from the work queue.

I will try to find some workaround how to solve it.

@toolmantim
Copy link
Contributor

Perhaps this would do it?

diff --git a/lib/capybara-screenshot/rspec.rb b/lib/capybara-screenshot/rspec.rb
index cc51fb0..fa60d0e 100644
--- a/lib/capybara-screenshot/rspec.rb
+++ b/lib/capybara-screenshot/rspec.rb
@@ -88,6 +88,7 @@ RSpec.configure do |config|
     if Capybara::Screenshot::RSpec.add_link_to_screenshot_for_failed_examples
       RSpec.configuration.formatters.each do |formatter|
         next unless (reporter_module = Capybara::Screenshot::RSpec::REPORTERS[formatter.class.to_s])
+        next if formatter.singleton_class.included_modules.include?(reporter_module)
         formatter.singleton_class.send :include, reporter_module
       end
     end

@ArturT
Copy link
Contributor Author

ArturT commented Mar 31, 2017

Perfect! It works. I will create PR for capybara-screenshot.

@toolmantim
Copy link
Contributor

Awesome!

ArturT added a commit to ArturT/capybara-screenshot that referenced this pull request Mar 31, 2017
before(:suite) is run multiple times because of using RSpec::Core::Runner

Related issue: buildkite/docs#99
@ArturT
Copy link
Contributor Author

ArturT commented Mar 31, 2017

I've created PR to capybara-screenshot. mattheworiordan/capybara-screenshot#205

@toolmantim
Copy link
Contributor

Awesome! I've just tested and it works AOK.

I also updated the gist:
https://gist.github.com/toolmantim/2a1d73fd2f8cca15f38255995af035a2

Will test that out on our app now.

@ArturT
Copy link
Contributor Author

ArturT commented Mar 31, 2017

I update the documentation for knapsack_pro in Queue Mode section and in FAQ.

I also updated examples rails apps:
https://github.com/KnapsackPro/buildkite-rails-docker-parallel-example-with-knapsack_pro/blob/master/spec/spec_helper.rb
https://github.com/KnapsackPro/buildkite-rails-parallel-example-with-knapsack_pro/blob/master/spec/spec_helper.rb

This should be good enough solution for now until capybara-screenshot fix is merged and a new gem version released. Then I will update docs for knapsack_pro to ensure the proper version of capybara-screenshot is in use.

mattheworiordan pushed a commit to mattheworiordan/capybara-screenshot that referenced this pull request Apr 3, 2017
before(:suite) is run multiple times because of using RSpec::Core::Runner

Related issue: buildkite/docs#99
@ArturT
Copy link
Contributor Author

ArturT commented Apr 6, 2017

@toolmantim It seems capybara-screenshot releases are not that often. I think we could merge this PR and I will keep an eye on the capybara-screenshot and when the release will be published then I will update documentation in buildkite-rails-parallel-example repos to point directly to capybara-screenshot version >= with our fix.

@toolmantim
Copy link
Contributor

Thanks @ArturT. This PR needs some changes before merging. We'll get to it shortly, I promise!

@ArturT
Copy link
Contributor Author

ArturT commented May 3, 2017

@toolmantim Please let me know if I need to change anything in this PR or here buildkite/example-pipelines#4

@toolmantim
Copy link
Contributor

toolmantim commented May 19, 2017

Thanks Artur! I've just updated it to match the rest of the style of our documentation, and will merge it now :) Sorry for the wait.

@toolmantim toolmantim merged commit cc0ad96 into buildkite:master May 19, 2017
@ArturT ArturT deleted the knapsack branch May 19, 2017 09:07
@ArturT
Copy link
Contributor Author

ArturT commented May 19, 2017

@toolmantim Looks awesome. Thanks!
There is one more left PR: buildkite/example-pipelines#4

@toolmantim
Copy link
Contributor

Thanks @ArturT. I'm not sure about merging that, because it's the only example that requires a commercial tool, and might be a little confusing. I wonder if we should simply update the Readme on the existing Knapsack example repo to point to the two Knapsack Pro example pipelines?

@toolmantim
Copy link
Contributor

Sorry for sitting on that one for a while… I had to figure out what to do! That's been fixed up on the other PR now.

@ArturT
Copy link
Contributor Author

ArturT commented May 22, 2017

Sure 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants