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

Initialize failure line groups as array. Issue #56 #66

Merged

Conversation

numbata
Copy link
Contributor

@numbata numbata commented Jan 22, 2020

Failure line groups should be initialized as array in case of Rspec's ExceptionPresenter.

Fixes #56 issue.

@numbata numbata requested a review from mcmire January 22, 2020 09:57
Copy link
Collaborator

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Thanks for this. So what's weird is that all of the integration tests like the ones here pass, so I'm curious what's different about these tests vs. the test you wrote in which you managed to get this error. Can you add a new context to this file and use the existing tests there to create a new test that reproduces it? That way we know that your changes work.

@numbata numbata force-pushed the rspec_exception_presenter_fix-issue-56 branch from 65d506c to 3ae5216 Compare January 27, 2020 00:47
@numbata
Copy link
Contributor Author

numbata commented Jan 27, 2020

@mcmire I added new spec with proof.

@numbata
Copy link
Contributor Author

numbata commented Jan 27, 2020

The exists integration tests do not cover the problem code inside "if". But this code can be executed in case of multiple exception error, that initiated with failure_lines option (Look here and around).

@numbata numbata requested a review from mcmire January 28, 2020 09:47
@mcmire
Copy link
Collaborator

mcmire commented Jan 28, 2020

Thanks! I'll take a look later today. :)

Copy link
Collaborator

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

This makes sense. Thanks for the patch!

@mcmire mcmire merged commit aca3398 into splitwise:master Jan 29, 2020
@kzaitsev
Copy link

@mcmire Hello, can you please release a new gem version with this fix?

@mcmire
Copy link
Collaborator

mcmire commented Jan 30, 2020

@Bugagazavr Yes! v0.4.1 is now out with this change.

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.

no implicit conversion of Symbol into Integer
3 participants