Fix #401 w/ integration spec for omniauth-oauth2. #402

Merged
merged 2 commits into from Feb 10, 2017

Conversation

Projects
None yet
4 participants
@dblock
Contributor

dblock commented Feb 10, 2017

Don't merge this, trying to reproduce #401.

@dblock dblock referenced this pull request Feb 10, 2017

Closed

Issue with omniauth-oauth2 #401

@matthewrudy

This comment has been minimized.

Show comment
Hide comment
@matthewrudy

matthewrudy Feb 10, 2017

Contributor

@dblock this doesn't fail because it doesn't use Rails.

Contributor

matthewrudy commented Feb 10, 2017

@dblock this doesn't fail because it doesn't use Rails.

@michaelherold

This comment has been minimized.

Show comment
Hide comment
@michaelherold

michaelherold Feb 10, 2017

Contributor

There's one extra caveat that I want to test and that's to make sure that we don't log anything when Rails starts.

The problem we're running into is that the Hashie logger initializes to a STDOUT logger. Then OmniAuth loads and has that overriding key which, for some reason, is getting through the state check for #disable_warnings?. That means there's one line logged to STDOUT before the Rails initializer kicks in.

Ugh. Meta-programming.

Contributor

michaelherold commented Feb 10, 2017

There's one extra caveat that I want to test and that's to make sure that we don't log anything when Rails starts.

The problem we're running into is that the Hashie logger initializes to a STDOUT logger. Then OmniAuth loads and has that overriding key which, for some reason, is getting through the state check for #disable_warnings?. That means there's one line logged to STDOUT before the Rails initializer kicks in.

Ugh. Meta-programming.

@dblock

This comment has been minimized.

Show comment
Hide comment
@dblock

dblock Feb 10, 2017

Contributor

This build actually fails, there's the error we expect in https://travis-ci.org/intridea/hashie/jobs/200526414, but it's not bubbled up :( Not sure why (yet).

Contributor

dblock commented Feb 10, 2017

This build actually fails, there's the error we expect in https://travis-ci.org/intridea/hashie/jobs/200526414, but it's not bubbled up :( Not sure why (yet).

@dblock

This comment has been minimized.

Show comment
Hide comment
@dblock

dblock Feb 10, 2017

Contributor

@michaelherold Your spec in #405 doesn't fail the same way as the original issue. This one is giving this, as expected:

/home/travis/build/intridea/hashie/lib/hashie/mash.rb:334:in `log_built_in_message': private method `warn' called for nil:NilClass (NoMethodError)
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:139:in `custom_writer'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:207:in `block in deep_update'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:200:in `each_pair'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:200:in `deep_update'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:115:in `initialize'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth/auth_hash.rb:24:in `new'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth/auth_hash.rb:24:in `regular_writer'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:140:in `custom_writer'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:207:in `block in deep_update'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:200:in `each_pair'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:200:in `deep_update'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:115:in `initialize'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth.rb:45:in `new'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth.rb:45:in `defaults'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth.rb:50:in `initialize'
	from /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/singleton.rb:142:in `new'
	from /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/singleton.rb:142:in `block in instance'
	from /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/singleton.rb:140:in `synchronize'
	from /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/singleton.rb:140:in `instance'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth.rb:119:in `config'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-oauth2-1.4.0/lib/omniauth/strategies/oauth2.rb:127:in `<top (required)>'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-oauth2-1.4.0/lib/omniauth-oauth2.rb:2:in `require'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-oauth2-1.4.0/lib/omniauth-oauth2.rb:2:in `<top (required)>'
	from /home/travis/build/intridea/hashie/spec/integration/omniauth-oauth2/some_site.rb:1:in `require'
	from /home/travis/build/intridea/hashie/spec/integration/omniauth-oauth2/some_site.rb:1:in `<top (required)>'
	from /home/travis/build/intridea/hashie/spec/integration/omniauth-oauth2/integration_spec.rb:9:in `require'
	from /home/travis/build/intridea/hashie/spec/integration/omniauth-oauth2/integration_spec.rb:9:in `<top (required)>'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1435:in `load'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1435:in `block in load_spec_files'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1433:in `each'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1433:in `load_spec_files'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:100:in `setup'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:86:in `run'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:71:in `run'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:45:in `invoke'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/exe/rspec:4:in `<top (required)>'
	from /home/travis/.rvm/gems/ruby-2.3.1/bin/rspec:23:in `load'
	from /home/travis/.rvm/gems/ruby-2.3.1/bin/rspec:23:in `<main>'
	from /home/travis/.rvm/gems/ruby-2.3.1/bin/ruby_executable_hooks:15:in `eval'
	from /home/travis/.rvm/gems/ruby-2.3.1/bin/ruby_executable_hooks:15:in `<main>'

I fixed the build to fail on integration specs too.

Contributor

dblock commented Feb 10, 2017

@michaelherold Your spec in #405 doesn't fail the same way as the original issue. This one is giving this, as expected:

/home/travis/build/intridea/hashie/lib/hashie/mash.rb:334:in `log_built_in_message': private method `warn' called for nil:NilClass (NoMethodError)
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:139:in `custom_writer'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:207:in `block in deep_update'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:200:in `each_pair'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:200:in `deep_update'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:115:in `initialize'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth/auth_hash.rb:24:in `new'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth/auth_hash.rb:24:in `regular_writer'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:140:in `custom_writer'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:207:in `block in deep_update'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:200:in `each_pair'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:200:in `deep_update'
	from /home/travis/build/intridea/hashie/lib/hashie/mash.rb:115:in `initialize'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth.rb:45:in `new'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth.rb:45:in `defaults'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth.rb:50:in `initialize'
	from /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/singleton.rb:142:in `new'
	from /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/singleton.rb:142:in `block in instance'
	from /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/singleton.rb:140:in `synchronize'
	from /home/travis/.rvm/rubies/ruby-2.3.1/lib/ruby/2.3.0/singleton.rb:140:in `instance'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-1.4.1/lib/omniauth.rb:119:in `config'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-oauth2-1.4.0/lib/omniauth/strategies/oauth2.rb:127:in `<top (required)>'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-oauth2-1.4.0/lib/omniauth-oauth2.rb:2:in `require'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/omniauth-oauth2-1.4.0/lib/omniauth-oauth2.rb:2:in `<top (required)>'
	from /home/travis/build/intridea/hashie/spec/integration/omniauth-oauth2/some_site.rb:1:in `require'
	from /home/travis/build/intridea/hashie/spec/integration/omniauth-oauth2/some_site.rb:1:in `<top (required)>'
	from /home/travis/build/intridea/hashie/spec/integration/omniauth-oauth2/integration_spec.rb:9:in `require'
	from /home/travis/build/intridea/hashie/spec/integration/omniauth-oauth2/integration_spec.rb:9:in `<top (required)>'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1435:in `load'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1435:in `block in load_spec_files'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1433:in `each'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/configuration.rb:1433:in `load_spec_files'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:100:in `setup'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:86:in `run'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:71:in `run'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/lib/rspec/core/runner.rb:45:in `invoke'
	from /home/travis/.rvm/gems/ruby-2.3.1/gems/rspec-core-3.5.4/exe/rspec:4:in `<top (required)>'
	from /home/travis/.rvm/gems/ruby-2.3.1/bin/rspec:23:in `load'
	from /home/travis/.rvm/gems/ruby-2.3.1/bin/rspec:23:in `<main>'
	from /home/travis/.rvm/gems/ruby-2.3.1/bin/ruby_executable_hooks:15:in `eval'
	from /home/travis/.rvm/gems/ruby-2.3.1/bin/ruby_executable_hooks:15:in `<main>'

I fixed the build to fail on integration specs too.

@dblock dblock changed the title from Added integration spec for omniauth-oauth2. to Fix #401 w/ integration spec for omniauth-oauth2. Feb 10, 2017

@@ -28,6 +28,7 @@ scheme are considered to be bugs.
### Fixed
+* [#402](https://github.com/intridea/hashie/pull/402): Use a Railtie to set Hashie.logger on rails boot - [@matthewrudy](https://github.com/matthewrudy).

This comment has been minimized.

@dblock

dblock Feb 10, 2017

Contributor

I moved this in the right place.

@dblock

dblock Feb 10, 2017

Contributor

I moved this in the right place.

@dblock

This comment has been minimized.

Show comment
Hide comment
@dblock

dblock Feb 10, 2017

Contributor

💚

Contributor

dblock commented Feb 10, 2017

💚

@michaelherold

This comment has been minimized.

Show comment
Hide comment
@michaelherold

michaelherold Feb 10, 2017

Contributor

This is fantastic, thank you @matthewrudy!

I'm going to investigate the logging to STDOUT before the Rails initializer kicks in this evening. It might just have been something that I was doing in my testing.

Contributor

michaelherold commented Feb 10, 2017

This is fantastic, thank you @matthewrudy!

I'm going to investigate the logging to STDOUT before the Rails initializer kicks in this evening. It might just have been something that I was doing in my testing.

@michaelherold michaelherold merged commit aef1b46 into intridea:master Feb 10, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@chulkilee

This comment has been minimized.

Show comment
Hide comment
@chulkilee

chulkilee Feb 11, 2017

Can we release new version to include this fix?

Can we release new version to include this fix?

@michaelherold

This comment has been minimized.

Show comment
Hide comment
@michaelherold

michaelherold Feb 11, 2017

Contributor

Yes, there's one more fix (#406) that needs reviewing and pulled in before we cut another release.

Contributor

michaelherold commented Feb 11, 2017

Yes, there's one more fix (#406) that needs reviewing and pulled in before we cut another release.

jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Mar 20, 2017

taca
Update ruby-hashie to 3.5.5.
# Change Log

## [3.5.5] - 2017-02-24

[3.5.5]: intridea/hashie@v3.5.4...v3.5.5

### Added

* [#326](intridea/hashie#326): Added `Hashie::Extensions::Mash::KeepOriginalKeys` to give Mashes the ability to keep the original structure given to it - [@michaelherold](https://github.com/michaelherold).

### Fixed

* [#415](intridea/hashie#415): Fixed Mash logging keys multiple times which lead to a bad user experience or, in some cases, errors - [@michaelherold](https://github.com/michaelherold).

## [3.5.4] - 2017-02-22

[3.5.4]: intridea/hashie@v3.5.3...v3.5.4

### Added

* [#412](intridea/hashie#412): Added a Hashie::Extensions::Mash::SymbolizeKeys extension that overrides the default stringification behavior for keys - [@michaelherold](https://github.com/michaelherold).

### Fixed

* [#409](intridea/hashie#409): Fixed Railtie detection for projects where Rails is defined but Railties are not availble - [@CallumD](https://github.com/callumd).
* [#411](intridea/hashie#411): Fixed a performance regression from 3.4.3 that caused a 10x slowdown in OmniAuth - [@michaelherold](https://github.com/michaelherold).

## [3.5.3] - 2017-02-11

[3.5.3]: intridea/hashie@v3.5.2...v3.5.3

### Fixed

* [#402](intridea/hashie#402): Use a Railtie to set Hashie.logger on rails boot - [@matthewrudy](https://github.com/matthewrudy).
* [#406](intridea/hashie#406): Ensure that subclasses that disable warnings propagate that setting to grandchild classes - [@michaelherold](https://github.com/michaelherold).
* Your contribution here.

## [3.5.2] - 2017-02-10

[3.5.2]: intridea/hashie@v3.5.1...v3.5.2

### Added

* [#395](intridea/hashie#395): Add the ability to disable warnings in Mash subclasses - [@michaelherold](https://github.com/michaelherold).
* [#400](intridea/hashie#400): Fix Hashie.logger load and set the Hashie logger to the Rails logger in a Rails environment - [@michaelherold](https://github.com/michaelherold).

### Fixed

* [#396](intridea/hashie#396): Fix for specs in #381: Incorrect use of shared context meant example was not being run - [@biinari](https://github.com/biinari).
* [#399](intridea/hashie#399): Fix passing Pathname object to Hashie::Mesh.load() - [@albb0920](https://github.com/albb0920).

### Miscellanous

* [#397](intridea/hashie#397): Add the integration specs harness into the main test tasks - [@michaelherold](https://github.com/michaelherold).

## [3.5.1] - 2017-01-31

* [#392](intridea/hashie#392): Fix for #391: Require all dependencies of Hashie::Mash - [@dblock](https://github.com/dblock).

[3.5.1]: intridea/hashie@v3.5.0...v3.5.1

## [3.5.0] - 2017-01-31

* [#386](intridea/hashie#386): Fix for #385: Make `deep_merge` always `deep_dup` nested hashes before merging them in so that there are no shared references between the two hashes being merged. - [@mltsy](https://github.com/mltsy).
* [#389](intridea/hashie#389): Support Ruby 2.4.0 - [@camelmasa](https://github.com/camelmasa).

[3.5.0]: intridea/hashie@v3.4.6...v3.5.0

### Added

* [#381](intridea/hashie#381): Add a logging layer that lets us report potential issues to our users. As the first logged issue, report when a `Hashie::Mash` is attempting to overwrite a built-in method, since that is one of our number one questions - [@michaelherold](https://github.com/michaelherold).

### Changed

* [#384](intridea/hashie#384): Updated to CodeClimate 1.x - [@boffbowsh](https://github.com/boffbowsh).

### Fixed

* [#369](intridea/hashie#369): If a translation for a property exists when using IndifferentAccess and IgnoreUndeclared, use the translation to find the property - [@whitethunder](https://github.com/whitethunder).
* [#376](intridea/hashie#376): Leave string index unchanged if it can't be converted to integer for Array#dig - [@sazor](https://github.com/sazor).
* [#377](intridea/hashie#377): Dont use Rubygems to check ruby version - [@sazor](https://github.com/sazor).
* [#378](intridea/hashie#378): Deep find all searches inside all nested hashes - [@sazor](https://github.com/sazor).
* [#380](intridea/hashie#380): Evaluate procs default values of Dash in object initialization - [@sazor](https://github.com/sazor).

### Miscellanous

* [#387](intridea/hashie#387): Fix builds failing due to Rake 11 having a breaking change - [@michaelherold](https://github.com/michaelherold).

@dblock dblock deleted the dblock:omniauth-oauth2 branch Mar 19, 2018

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