Skip to content

[WIP] @test_reference: add the return_bool keyword argument to get the pass/fail status as a Bool #111

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

Closed
wants to merge 3 commits into from

Conversation

DilumAluthge
Copy link
Member

No description provided.

Comment on lines 120 to 125
if haskey(kw, :return_bool)
return_bool = kw[:return_bool]::Bool
else
return_bool = false
end
kw = filter(p -> p[1] != :return_bool, kw)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a suggestion for simpler code

Suggested change
if haskey(kw, :return_bool)
return_bool = kw[:return_bool]::Bool
else
return_bool = false
end
kw = filter(p -> p[1] != :return_bool, kw)
return_bool = get(kw, :return_bool, false)::Bool
delete!(kw, :return_bool)

Copy link
Member Author

Choose a reason for hiding this comment

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

The return_bool = get(kw, :return_bool, false)::Bool part is good.

The delete! throws a MethodError.

@IanButterworth
Copy link
Collaborator

This might be better as a @check_reference macro, or something like that?
@test... is kind of tied into the Test architecture?

Not a strong opinion

@johnnychen94
Copy link
Member

johnnychen94 commented Apr 30, 2022

This is a good change but might need some refactoring work. We've discussed changing the interface to match_reference(x, ref; kw...)::Bool would be a satisfying solution: that people can use normal Tests utilities to do the common work, e.g.,

@test match_reference(line, "this is the reference line")
@test_broken match_reference(line, "this is another line")

@DilumAluthge
Copy link
Member Author

Sure, I can refactor the code out into a match_reference function, and then test_reference would call match_reference under the hood.

@DilumAluthge
Copy link
Member Author

#112

@DilumAluthge DilumAluthge deleted the dpa/return-bool branch May 11, 2022 03:11
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.

3 participants