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

Remove deprecated method #1321

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@orien
Contributor

orien commented Oct 5, 2018

Summary

Fix two tests that were failing.

Details

The test suite was failing with:

  1) Cucumber::Formatter::Interceptor::Pipe#unwrap! disables the pipe bypass
     Failure/Error: raise "This method is due for removal after version #{remove_after_version}" if Cucumber::VERSION > remove_after_version

     RuntimeError:
       This method is due for removal after version 3.99
     # ./lib/cucumber/deprecate.rb:20:in `call'
     # ./lib/cucumber/deprecate.rb:28:in `deprecate'
     # ./lib/cucumber/formatter/interceptor.rb:24:in `buffer'
     # ./spec/cucumber/formatter/interceptor_spec.rb:88:in `block (3 levels) in <module:Formatter>'

  2) Cucumber::Formatter::Interceptor::Pipe#write adds the buffer to its stored output
     Failure/Error: raise "This method is due for removal after version #{remove_after_version}" if Cucumber::VERSION > remove_after_version

     RuntimeError:
       This method is due for removal after version 3.99
     # ./lib/cucumber/deprecate.rb:20:in `call'
     # ./lib/cucumber/deprecate.rb:28:in `deprecate'
     # ./lib/cucumber/formatter/interceptor.rb:24:in `buffer'
     # ./spec/cucumber/formatter/interceptor_spec.rb:112:in `block (3 levels) in <module:Formatter>'

The method Cucumber::Formatter::Interceptor::Pipe#buffer has been scheduled for removal before version 4.0.0 is released.

I removed this method and migrated tests to use the replacement method: Cucumber::Formatter::Interceptor::Pipe#buffer_string.

Motivation and Context

This moves us one step closer to having a passing CI build.

How Has This Been Tested?

The method has been removed, allowing tests to pass.

Screenshots (if appropriate):

Types of changes

  • Refactor (code change that does not change external functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I've added tests for my code
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
@@ -20,7 +14,7 @@ module Cucumber::Formatter
context 'when passed :stderr' do
before :each do
@stderr = $stdout
@stderr = $stderr

This comment has been minimized.

@orien

orien Oct 15, 2018

Contributor

We should save $stderr so we can put it back how we found it after the spec run.

@orien

orien Oct 15, 2018

Contributor

We should save $stderr so we can put it back how we found it after the spec run.

This comment has been minimized.

@luke-hill

luke-hill Oct 15, 2018

We don't need an each declaration when we're inside an area with one test. But that's probably splitting hairs.

@luke-hill

luke-hill Oct 15, 2018

We don't need an each declaration when we're inside an area with one test. But that's probably splitting hairs.

pipe
end
let(:pipe) { instance_spy(IO) }

This comment has been minimized.

@orien

orien Oct 15, 2018

Contributor

We can use a simple instance spy here.

@orien

orien Oct 15, 2018

Contributor

We can use a simple instance spy here.

@koic koic referenced this pull request Oct 15, 2018

Merged

Fix rubocop backlog issues #1322

0 of 6 tasks complete
@@ -20,7 +14,7 @@ module Cucumber::Formatter
context 'when passed :stderr' do
before :each do
@stderr = $stdout
@stderr = $stderr

This comment has been minimized.

@luke-hill

luke-hill Oct 15, 2018

We don't need an each declaration when we're inside an area with one test. But that's probably splitting hairs.

@luke-hill

luke-hill Oct 15, 2018

We don't need an each declaration when we're inside an area with one test. But that's probably splitting hairs.

@jaysonesmith

This comment has been minimized.

Show comment
Hide comment
@jaysonesmith

jaysonesmith Oct 15, 2018

Member

@enkessler Looks like the Windows builds are failing on some formatting stuff for the /branch build. The /pr build looks to have jumped the gun on the exe's being available or something along those lines? I can't retry the latter, so I'm not sure if it'd end up failing for the same reason as the /branch one.

Any idea here?

Member

jaysonesmith commented Oct 15, 2018

@enkessler Looks like the Windows builds are failing on some formatting stuff for the /branch build. The /pr build looks to have jumped the gun on the exe's being available or something along those lines? I can't retry the latter, so I'm not sure if it'd end up failing for the same reason as the /branch one.

Any idea here?

@orien

This comment has been minimized.

Show comment
Hide comment
@orien

orien Oct 17, 2018

Contributor

The Windows build failures are affecting the master branch also.
https://ci.appveyor.com/project/enkessler/cucumber-ruby-u5iqh/build/job/dxauxu8fnx24vcrq

The problem is not introduced by this PR.

Contributor

orien commented Oct 17, 2018

The Windows build failures are affecting the master branch also.
https://ci.appveyor.com/project/enkessler/cucumber-ruby-u5iqh/build/job/dxauxu8fnx24vcrq

The problem is not introduced by this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment