-
Notifications
You must be signed in to change notification settings - Fork 125
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
[Fix bug] modify assert_line_text method to be able to make correct assertions #71
Conversation
test/support/utils.rb
Outdated
line.include? expected | ||
result = false | ||
@internal_info['backlog'].each do |line| | ||
if line.include? expected |
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.
can you use match?
instead of include?
so it can also take regexp as expectation?
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.
1b99dd2
to
e97c30b
Compare
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 👍
test/support/utils.rb
Outdated
end | ||
end | ||
assert_true(result) |
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.
It should show failure message. When the test fails, there is no hints and it is difficult to debug.
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, I will add it.
e97c30b
to
d747c12
Compare
d747c12
to
d4fe7ba
Compare
test/support/utils.rb
Outdated
assert_block do | ||
@internal_info['backlog'].each do |line| | ||
line.include? expected | ||
msg = "Expected to include #{expected}}" |
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.
it still doesn't provide much information:
Expected to include => 20}.
<true> expected but was
<false>
how about this?
assert_match(expected, @internal_info['backlog'].join)
</=>\ 20/> was expected to be =~
<"=>#0\tFoo#first_call at /var/folders/yg/hnbymwxd5pn7v94_clc59y6r0000gn/T/debugger20210608-65153-b857w.rb:4 #=> 30\n" +
" #1\t<main> at /var/folders/yg/hnbymwxd5pn7v94_clc59y6r0000gn/T/debugger20210608-65153-b857w.rb:23\n">.
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.
That's a good idea! Thanks 👍
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.
d4fe7ba
to
518ba5b
Compare
btw it'd be great if we can merge this sooner than later because the current matcher doesn't take regexp object and never fails when passing string 😬 |
No description provided.