Skip to content

Make puma log silence completely #2011

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Make puma log silence completely #2011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc80fac
Make puma log silence completely
ta1kt0me Jul 14, 2018
3ad30b8
Enable chrome on travisci
ta1kt0me Jul 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
language: ruby

sudo: false
sudo: required
addons:
chrome: stable
apt:
packages:
- chromium-chromedriver

# We deviate from the rspec-dev cache setting here.
#
Expand Down Expand Up @@ -31,6 +36,7 @@ before_script:
# Rails 4.x complains with a bundler rails binstub in PROJECT_ROOT/bin/
- rm -f bin/rails
- bundle exec rails --version
- ln -s /usr/lib/chromium-browser/chromedriver ~/bin/chromedriver

script: "script/run_build 2>&1"

Expand Down
2 changes: 1 addition & 1 deletion Rakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ namespace :generate do

# Rails 4 cannot use a `rails` binstub generated by Bundler
sh "rm -f #{bindir}/rails"
sh "bundle exec rails new ./tmp/example_app --no-rc --skip-javascript --skip-sprockets --skip-git --skip-test-unit --skip-listen --skip-bundle --template=example_app_generator/generate_app.rb"
sh "bundle exec rails new ./tmp/example_app --no-rc --skip-javascript --skip-git --skip-test-unit --skip-listen --skip-bundle --template=example_app_generator/generate_app.rb"
Copy link
Member

@benoittgt benoittgt Dec 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why you changed this?

This has a cost. It seems that sprockets adds sass gem. On Ruby 1.9.3 it adds deprecation warning and breaks the CI

     Failure/Error:
       expect(capture_exec(custom_env, exec_script)).to eq(
         "#{::Rails.root}/spec/mailers/previews"
       )
     
       expected: "/home/travis/build/rspec/rspec-rails/tmp/example_app/spec/mailers/previews"
            got: #<struct CaptureExec io="DEPRECATION WARNING:\nSass 3.5 will no longer support Ruby 1.9.3.\nPlease up...ible.\n\n/home/travis/build/rspec/rspec-rails/tmp/example_app/spec/mailers/previews", exit_status=0>

I revert this line and test on Ruby 1.9.3 without issue. I didn't test on all rails version. But maybe it doesn't need to be committed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I'd like this reverted too


in_example_app do
sh "./travis_retry_bundle_install.sh 2>&1"
Expand Down
28 changes: 27 additions & 1 deletion features/system_specs/system_specs.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Feature: System spec


@system_test
Scenario: System specs
Scenario: System specs driven by rack_test
Given a file named "spec/system/widget_system_spec.rb" with:
"""ruby
require "rails_helper"
Expand All @@ -33,3 +33,29 @@ Feature: System spec
When I run `rspec spec/system/widget_system_spec.rb`
Then the exit status should be 0
And the output should contain "1 example, 0 failures"

@system_test
Scenario: System specs driven by selenium_chrome_headless
Given a file named "spec/system/widget_system_spec.rb" with:
"""ruby
require "rails_helper"

RSpec.describe "Widget management", :type => :system do
before do
driven_by(:selenium_chrome_headless)
end

it "enables me to create widgets" do
visit "/widgets/new"

fill_in "Name", :with => "My Widget"
click_button "Create Widget"

expect(page).to have_text("Widget was successfully created.")
end
end
"""
When I run `rspec spec/system/widget_system_spec.rb`
Then the exit status should be 0
And the output should contain "1 example, 0 failures"
And the output should not contain "Puma starting"
6 changes: 5 additions & 1 deletion lib/rspec/rails/example/system_example_group.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ def app
end

included do |other|
ActiveSupport.on_load(:action_dispatch_system_test_case) do
ActionDispatch::SystemTesting::Server.silence_puma = true
end

begin
require 'capybara'
require 'action_dispatch/system_test_case'
Expand All @@ -71,7 +75,7 @@ def app

attr_reader :driver

if ActionDispatch::SystemTesting::Server.respond_to?(:silence_puma=)
if ::Rails.version.to_f == 5.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first implementation is lack of considering of compatibility. action_dispatch_system_test_case is available from Rails 5.2 . The change is needed to pass System specs driven by selenium_chrome_headless spec in Rails 5.1 .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was literally checking the method exists, then using it, can you explain the "lack of compatibility" I'm afraid I don't understand how "not calling the method if it doesn't exist" changes anything... Also note this need to work on 5.2 and beyond, so this check seems incorrect.

ActionDispatch::SystemTesting::Server.silence_puma = true
end

Expand Down