On 18/05/14 15:42, Tim Wright wrote:
>
> In 
> src/main/java/org/concordion/internal/runner/DefaultConcordionRunner.java:
>
> > @@ -48,17 +65,129 @@ public RunnerResult execute(Resource resource, String href) throws Exception {
> >          return concordionClass;
> >      }
> >
> > -    protected RunnerResult runTestClass(Class<?> concordionClass) throws Exception {
> > -        org.junit.runner.Result jUnitResult = runJUnitClass(concordionClass);
> > +    protected ResultSummary runTestClass(Class<?> concordionClass) throws Exception {
> > +    	// check for a jUnit 3 style test extension
> > +       	if (ConcordionTestCase.class.isAssignableFrom(concordionClass)) {
> > +    		return runConcordionJUnit3Test(concordionClass);
> > +    	}
> > +
> > +       	// check for a jUnit 4 style annotation
> > +       	if (concordionClass.isAnnotationPresent(RunWith.class)) {
> > +       		RunWith s = concordionClass.getAnnotation(RunWith.class);
> > +       		if (s.value().isAssignableFrom(ConcordionRunner.class)) {
>
> I think the annotated way is better - that way alternate runners can 
> be tested with the new code before using it. It also provides a way 
> for the alternates to "opt out" of the enhanced reporting. Will make 
> the change.
>
> I haven't tested the code with alternate runners - either things 
> extending ConcordionRunner or BlockJUnitClassRunner. Hopefully I've 
> got the code right - but it's tricky introspection and might have a 
> test around the wrong way.
>
> —
> Reply to this email directly or view it on GitHub 
> <https://github.com/concordion/concordion/pull/83/files#r12772014>.
>
Agreed.

I've also let Mark Barnes, the author of Concordion Plus, know about 
this work. He's going to have a look, and will probably try out the 
annotation.

The concordion:run work could definitely do with some additional tests, 
including extending the different runners. There's that whole chunk of 
code around passing parameters and variables that isn't well documented 
and doesn't have any tests.

cheers