-
Notifications
You must be signed in to change notification settings - Fork 57
Upgrade rspec version to 3.x #94
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
Conversation
This conversion is done by Transpec 3.4.0 with the following command:
transpec -f
* 70 conversions
from: obj.should
to: expect(obj).to
* 25 conversions
from: == expected
to: eq(expected)
* 4 conversions
from: obj.should_not
to: expect(obj).not_to
For more details: https://github.com/yujinakayama/transpec#supported-conversions
| begin | ||
| expect(b['hits'][0]['name']).to eq('"> hack0r') | ||
| expect(b['hits'][0]['name']).to eq('"> hack0r').and_raise(StandardError) | ||
| expect(b['hits'][0]['author']).to eq('alert(1)') | ||
| expect(b['hits'][0]['_formatted']['name']).to eq('"> <em>hack</em>0r') | ||
| rescue StandardError | ||
| # rails 4.2's sanitizer | ||
| begin | ||
| expect(b['hits'][0]['name']).to eq('"> hack0r') | ||
| expect(b['hits'][0]['name']).to eq('"> hack0r').and_raise(StandardError) | ||
| expect(b['hits'][0]['author']).to eq('') | ||
| expect(b['hits'][0]['_formatted']['name']).to eq('"> <em>hack</em>0r') | ||
| rescue StandardError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failures:
1) Book sanitizes attributes
Failure/Error: Transpec.analyze((expect(b['hits'][0]['name'])), self, "spec/integration_spec.rb_29236_29264", { :expect_source_location => [:context, "method(:expect).source_location"], :expect_example_method_defined_by_user? => [:context, "owner = method(:expect).owner\nowner != RSpec::Core::ExampleGroup &&\n owner.ancestors.include?(RSpec::Core::ExampleGroup)"] }).to eq('"> hack0r')
expected: "\"> hack0r"
got: "\"> hack0r"
(compared using ==)
It may require further investigation, not sure why this behaviour has changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are doing this kind of flow control with exceptions could be hard to tell what rails version was used in the first place. I'll reference your question into a issue and this problem in a new issue to be investigated in the future :)
| Exclude: | ||
| - 'lib/meilisearch-rails.rb' | ||
|
|
||
| # Offense count: 15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! 👏
brunoocasali
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jitingcn, thanks again for your contribution you rocks 🤘!
I left a comment about rubocop cop RSpec/MultipleExpectations, let me know what you think about that :)
Thanks for raising the issue and making this improvement!
| begin | ||
| expect(b['hits'][0]['name']).to eq('"> hack0r') | ||
| expect(b['hits'][0]['name']).to eq('"> hack0r').and_raise(StandardError) | ||
| expect(b['hits'][0]['author']).to eq('alert(1)') | ||
| expect(b['hits'][0]['_formatted']['name']).to eq('"> <em>hack</em>0r') | ||
| rescue StandardError | ||
| # rails 4.2's sanitizer | ||
| begin | ||
| expect(b['hits'][0]['name']).to eq('"> hack0r') | ||
| expect(b['hits'][0]['name']).to eq('"> hack0r').and_raise(StandardError) | ||
| expect(b['hits'][0]['author']).to eq('') | ||
| expect(b['hits'][0]['_formatted']['name']).to eq('"> <em>hack</em>0r') | ||
| rescue StandardError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we are doing this kind of flow control with exceptions could be hard to tell what rails version was used in the first place. I'll reference your question into a issue and this problem in a new issue to be investigated in the future :)
225d271 to
f2325c5
Compare
brunoocasali
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, we're good to go!
Thank you @jitingcn 🥇
curquiza
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot, @jitingcn for your PR! It definitely improves this repository!
|
bors merge |
|
Build succeeded: |
Fixes #93