Skip to content

allow passing in context information to assert methods#9

Open
haarg wants to merge 1 commit into
houseabsolute:masterfrom
haarg:exception-context
Open

allow passing in context information to assert methods#9
haarg wants to merge 1 commit into
houseabsolute:masterfrom
haarg:exception-context

Conversation

@haarg

@haarg haarg commented Feb 16, 2017

Copy link
Copy Markdown
Contributor

This isn't meant to be a finished product, but a starting point for discussion.

What I'd like to be able to do is attach attribute information when doing assertions, and have that information end up in the exception object and stringification.

@autarch autarch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks pretty reasonable. I had a few comments about the details.

Comment thread lib/Specio/Exception.pm
init_arg => undef,
},
context => {
isa => 'HashRef',

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if this really needs to be a HashRef. Is there any reason not to allow any defined value? I guess this would make including it in the as_string output harder, but as it stands this output won't be very helpful if the hashref contains any references of its own.

Comment thread lib/Specio/Exception.pm
if ($self->context and my %context = %{$self->context}) {
$str
.= ' ('
. join(', ', map { "$_: $context{$_}" } sort keys %context)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could just use Specio::PartialDump here, which would make even more sense if this wasn't required to be a hashref.

Comment thread t/exception.t
};

like(
$e->as_string,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It'd be good to also test that $e->context contains what we expect.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants