Test-More / Test2-Harness

Alternative to Test::Harness
Other
23 stars 26 forks source link

Allow plugins to specify custom job runners on self-added files #113

Closed ehuelsmann closed 5 years ago

ehuelsmann commented 5 years ago

By specifying the 'via' attribute on instances of Test2::Harness::Util::TestFile returned from $plugin->find_files(), this change allows plugins to override the value selected by default in Test2::Harness::Run::Runner. I.e. this change allows specification of a value for '$task->{via}' in Test2::Harness::Run::Runner::run_job.

ehuelsmann commented 5 years ago

Please provide your comment to the following thought: If we make the via attribute of Test2::Harness::Util::TestFile read/writable, then plugins can decide to set the attribute in the munge_files call. That would be good use of the existing plugin structure, but a rather obfuscated solution to a good use-case and would definitely call for a How do I port my SourceHandler to Yath/Test2::Harness type of manual.

Another line of thought would be that the above requires 'cooperation' of plugins: instead of writing a value to via, they would need to append to it. With a specific plugin callback (run_via), that can be circumvented, because each plugin will be able to list which job runners it elects to provide for certain scripts. I could PR this idea as an extension to the current PR or as a separate PR to show the associated code.

exodist commented 5 years ago

Aha! I knew I had done something like this before. We do not need to make 'via' writable. munge_files() gets an arrayref. At work I use it this way:

sub munge_files {
    my $plugin = shift;
    my ($files) = @_;

    @$files = map { Test2::Harness::Util::TestFile->new(%{$_}, via => 'Custom') } @$files;

    return;
}

The TestFile instance should never be modified, but nothing stops you from replacing them with modified copies. It may be a silly distinction, but the field was intended to be immutable. munge_files uses the arrayref instead of a list you cannot modify for this reason.

ehuelsmann commented 5 years ago

Ok. Then this PR by itself enables the 'via' accessor which provide the plugins with the current value for the TestFile instances as well as the code to pass the 'via' value on to the queue_item.

What remains is the work to create the plugin which uses this mechanism and the writing of documentation. The former is part of another project. Do you want the latter in this PR, or is it acceptable as it is?

exodist commented 5 years ago

oops, I realized I did not give you to "proper" invocation above

sub munge_files {
    my $plugin = shift;
    my ($files) = @_;

    @$files = map { Test2::Harness::Util::TestFile->new(%{$_}, queue_args => [via => ['Custom']]) } @$files;

    return;
}

No patch is needed.

exodist commented 5 years ago
exodist commented 5 years ago

Another correction, that invocation should probably merge in any existing data in queue_args if it is already present.

ehuelsmann commented 5 years ago

Ok. Thanks for the review. I'll withdraw the PR then; docs will need their own PR.

ehuelsmann commented 5 years ago

With the above clarifications, I was able to build a working prototype plugin+custom runner to run the Perl Cucumber tests (Test::BDD::Cucumber) that I want to switch over to Yath.

I'll keep this issue updated with my status.