Bug #92171 for AppConfig: ->can() does not respond properly to routines delegated to STATE

Wed Jan 15 10:17:40 2014 mlx [...] cpan.org - Ticket created

Subject:

->can() does not respond properly to routines delegated to STATE

Test fails: can_ok( $cfg, qw( define get set ) ); There are also other possibilities such as $cfg->varname, but those items listed in the documentation should at least pass. I believe that a can method in AppConfig that looks like this: sub can { my $self = shift; $self->SUPER::can( @_ ) || $self->{ STATE }->can( @_ ); } takes care of the issue, but I have not done fixing of this type of stuff in the past.

Mon Feb 15 12:37:34 2016 estrabd [...] gmail.com - Correspondence added

On Wed Jan 15 10:17:40 2014, midlifexis@wightmanfam.org wrote: Show quoted text

> Test fails: > > can_ok( $cfg, qw( define get set ) ); > > There are also other possibilities such as $cfg->varname, but those > items listed in the documentation should at least pass. > > I believe that a can method in AppConfig that looks like this: > > sub can { > my $self = shift; > $self->SUPER::can( @_ ) || $self->{ STATE }->can( @_ ); > } >

Hi, this is the behavior I would expect. Show quoted text

> takes care of the issue, but I have not done fixing of this type of > stuff in the past.

I suggest that this ticket is a non-issue. For starters, none of the tests use Test::More; in fact they use their own test harness - however it can be argued that this is another issue altogether. Secondly - and more importantly, t/state.t thoroughly tests the functionality of AppConfig::State. Testing to make sure that delegation works as advertised doesn't mean that delegation should be done in a way that would make Test::More::can_ok happy. If one were to fix this, it would be more prudent to create delegate wrappers in AppConfig rather than rely on AUTOLOAD. But, again, this sort of addition would be pointless and create more surface area for future bugs and incompatibilities. I do not suggest this is a reasonable solution either. My recommendation would be to close this ticket as not adding any value. Thank you.

Mon Feb 15 12:37:35 2016 The RT System itself - Status changed from 'new' to 'open'

Mon Feb 15 14:55:39 2016 mlx [...] cpan.org - Correspondence added

On Mon Feb 15 12:37:34 2016, ESTRABD wrote: Show quoted text

> I suggest that this ticket is a non-issue.

The can(...) function is returning false on the module even though the POD documentation indicates that the method tested with can() is supposed to be valid. If the method being queried is delegated or not is not a concern of whatever is calling can(). Show quoted text

> For starters, none of the tests use Test::More; in fact they use their > own test harness - however it can be argued that this is another issue > altogether.

This does not have anything to do with the testing harness used. Show quoted text

> Secondly - and more importantly, t/state.t thoroughly tests the > functionality of AppConfig::State. Testing to make sure that > delegation works as advertised doesn't mean that delegation should be > done in a way that would make Test::More::can_ok happy.

I never said that it should. All I stated was that a can() call against one of these methods returns false, even though, per documentation, the response should be true. Show quoted text

> If one were to fix this, it would be more prudent to create delegate > wrappers in AppConfig rather than rely on AUTOLOAD. But, again, this > sort of addition would be pointless and create more surface area for > future bugs and incompatibilities. I do not suggest this is a > reasonable solution either.

That would also handle the issue, but as I stated, delegation is an implementation concern, and does not matter to this ticket. Show quoted text

> My recommendation would be to close this ticket as not adding any > value.