Perl's not the only TAP source

[Steve Purkis]

On 20 Jun 2009, at 20:55, David E. Wheeler wrote:

> On Jun 20, 2009, at 6:14 AM, Steve Purkis wrote:
>
>> Nice - that's exactly the kinda thing I had in mind :-).
>
> Me too. I want to show off a released version of TAP::Harness/Parser
> running Perl, pgTAP, and JavaScript tests at OSCON next month.

Should be do-able, there's still a month.
Would have loved to go, only it's smack-bang in the middle of my  
holiday :-/


>> I take the point on not being forced to sub-class to write a plugin,
>> using roles is fine by me so long as we include an instantiation  
>> step.
>
> What for? I mean, what's the difference between calling functions in a
> module and passing an object vs. calling methods on objects? Requiring
> instantiation means source authors have to implement three methods.

Well, my reasons were mainly to accommodate common code (see below).   
Plus, TAP::Parser currently expects to get a source object back (but  
that's easy to change).  The back of my mind's saying something about  
storing state, but if we take away all the other methods that doesn't  
really apply.


>> That way it gives the best of both worlds - that way the internal
>> classes can all be implemented by sub-classing to share common code,
>> and the external plugins can do whatever they want.
>
> What common code? In the example I provided, the code was pretty
> damned simple. I pretty much removed all of the duplicate code.

Well, I was thinking we've gotta accommodate the internal source  
classes, and Perl inherits from Executable because they share code.   
And who's to say a plugin author wouldn't want to use inheritance to  
avoid code duplication?  Of course, you don't _need_ to instantiate  
for that, so it would still be possible to sub-class...  but it makes  
passing commonly used vars around a lot easier.

At any rate, the key point is: either we keep the instantiation, or we  
push that common code into a combo of the SourceFactory, the object  
that gets passed to the Source, and the 'Process' iterator.  Then we  
get rid of the need to store state in the Sources.  How all that'd be  
done is all a bit fuzzy to me, I'd need to think about it some more to  
see if it was feasible (should be, given it's now a black box to the  
Parser after my last round of refactoring).


>> As far as I can tell, there are 2 options:
>> * source & formatter plugins only (ie: source API stays the same)
>> * generic plugins (API not defined yet)
>
> Agreed. We already have pluggable formatters, no?

A limited form of...  see formatter_class & formatter in:

http://search.cpan.org/dist/Test-Harness/lib/TAP/Harness.pm#new

ie: no user config possible, unless you instantiate the formatter  
yourself and pass it to TAP::Harness.  Not ideal.  If you use  
formatter_class it does get auto-loaded though (I forgot about that in  
the last email).

I was thinking something similar to sources, eg:

	formatters => {
		Console => { foo => 'bar'},
		HTML => { baz => 'quux' },
	}

But that really belongs in a different thread (yes, I'd like to be  
able to have multiple formatters - can of worms).

...
>> 2. Figure out what to do with TAP::Parser::Iterators.
[snip]

You've convinced my with this & other emails.  Makes sense to keep  
iterators, but the factory class can go.


>> 3. Convert the Source & SourceFactory API to use roles.
>> At a glance, I like your suggestions (can_handle / source).
>
> can_handle/stream in my last proposal.

Oops, typo.

On second thoughts, 'stream' should prolly be 'iterator', or  
'make_iterator' to be consistent with the rest of the codebase.


>> But I'd need to think more about how the interface and make sure we
>> handle everything (source config, command params, sharing common
>> code, etc), before refactoring.
>
> There isn't a lot of common code; indeed, there's almost no code at
> all. That's the beautiful thing. The config, params, etc., can all be
> passed in via an object. Hell, if there is common code you can have it
> in methods on that object.

As above, there's common code internally, whatever we do needs to  
accommodate it.


>> I've had a quick think about the make_with_rawsource /
>> make_with_command idea, and realized that this would be pushing the
>> decision about what type of source you've got into the
>> SourceFactory. My first reaction is: that's asking it to do too
>> much.  The whole point of having source detectors was so that they
>> can make the decision.  Yes, it does do some detective work to
>> assemble meta data, but that was more to avoid code duplication, the
>> detectors can still ignore it if they choose.  My second reaction
>> is: that could simplify the API some, which is good for the plugin
>> writer.
>
> Not following you here. With my latest example, there is just a
> stream() function in the source that knows what kind of iterator to
> return. So the Raw source would return a raw iterator, while the PHP
> source would return an exec iterator.

Err.. You recommended make_with_command / make_with_rawsource at the  
end of your previous email... maybe better if you don't remember ;-)

I mis-read it anyways, you were talking about changing the iterator  
factory, not the source factory.  And if we ditch the iterator  
factory, then this no longer matters.

Still, it's important to get the roles & responsibilities right.  More  
on that in a sec.


>> My third though is your suggestion on passing objects through might
>> be the way to go.
>
> Yes, exactly.

This is probably the key - so what would that object look like?  At  
the moment, I'm thinking something ala:

RawSource
	raw_source_ref
	meta
	merge
	switches
	test_args

Another option I can see the outline of... instead of having  
raw_source_ref, it might be possible to have different types of  
RawSource classes?  array, file, scalar, hash ...  but then, you end  
up getting into that annoying scenario of premature source detection.   
Of course, for internal cases you usually already know what you've got  
so it could be more flexible.

Going back to roles & responsibilities.  This is how I'm starting to  
see them:

Parser:
	has a raw source & user config (eg: sources, merge) as input
	creates RawSource (if it doesn't already have one)
	uses a source factory to create a source
	uses source to create an iterator
	(ultimately only cares about iterators)

RawSource:
	the raw source
	params
	gathers & stores meta data
		(note to self: include shebang for Perl source)
	(could put all source config in here?)

SourceFactory:
	auto-loads source classes
	chooses best source to use, handing over:
		raw source
		source-specific config

Source:
	detects sources
	creates iterators

I think you mentioned in another email we might be better off naming  
the sources SourceDetectors again.  In a way that makes sense; if we  
go further down this route we could always shuffle the nomenclature:

	Source --> SourceDetector
	RawSource --> Source

We could also make TAP::Parser skip the source creation bit, and go  
straight to Iterators.  eg:

	SourceFactory --> IteratorFactory (replace existing one)

I'm not sure if either would be worth it though.  Does it really make  
their roles more clear?

We should at the very least replace all use of 'stream' with  
'iterator' in TAP::Parser.

...
>> 5. Deprecated API release.
>> Once we all agree on this, we should make a point release to
>> deprecate the API before we do the real release.  As this changes
>> the API quite a bit, is it worth bumping up the minor release number?
>
> I thought that the API was preserved, that this stuff was all in
> addition. No?

No - the TAP::Parser API will not be backwards compat.  Not sure if  
anyone was using anything this deep anyway (ie: sub-classing  
TAP::Parser, or TAP::Parser components), but I've marked the affected  
methods: grep -rin deprecated lib


>> Now the bad news as for when I'll be free...  I've got an exam to
>> prep for in a week and a half, so hopefully after that.
>
> As I said, I'd love to get this done by OSCON so I could show it off
> to all the Perl geeks there and make the Ruby, Python, and PHP geeks
> jealous!

It'd be great to demo @ OSCon, it'd be nice to show that TAP is really  
not just for Perl with examples to back it up.

Integrate it into BuildBot & Hudson, record historical stats & produce  
a funky UI and they'll get *really* jealous :)

Cheers,
-Steve