[Junit-trackers] [ junit-Bugs-2869568 ] ParentRunner filtering "internally temporarily" broken

[SourceForge.net <no...@so...>]

Bugs item #2869568, was opened at 2009-09-29 04:39
Message generated for change (Comment added) made by dsaff
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=115278&aid=2869568&group_id=15278

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: framework
Group: None
Status: Closed
Resolution: None
Priority: 5
Private: No
Submitted By: Reinhold Füreder (reinholdfuerede)
Assigned to: Nobody/Anonymous (nobody)
Summary: ParentRunner filtering "internally temporarily" broken

Initial Comment:
(I was not sure how to formulate the summary...)
This issue is also related to these JUnit bugs:
* "CompositeRunner.filter incorrect if child throws NoTestsRema" - ID: 1815188
* "Sorters and Filters are ignored in 4.5" - ID: 2008607

When using a filter on a composite test, i.e. ParentRunner and below, the filtering happens in two steps:
1. Partial filtering on composite test based on Filter class and Filterable interface
2. Full filtering when running the tests via ParentRunner class: childrenInvoker(), runChildren() and getFilteredChildren() methods

I think the behaviour of JUnit 4.3.1 and 4.4 was different and better in that the filtering was done in a single step.
The problems of the current approach are in my mind:
* The first filtering that throws the NoTestsRemainException is not complete, so it may happen that only the second filtering removes "silently" some more tests. This leads in my case to JUnit reports (via ant-junit) that contains test classes without test cases.
* Performance could be improved: as filtering is done multiple times and shouldRun() is additionally used vai getDescription() and sorting in getFilteredChildren())

As I think it was more or less suggested in one of the referenced bug reports above a quick "fix" (in fact a refactoring of the two step filtering should be strived for in my mind) is in ParentRunner.filter(Filter filter) method:

Old:

    public void filter(Filter filter) throws NoTestsRemainException {
    	fFilter= filter;
    
    	for (T each : getChildren())
    		if (shouldRun(each))
    			return;
    	throw new NoTestsRemainException();
    }

New:

    public void filter(Filter filter) throws NoTestsRemainException {
    	fFilter= filter;
    
        ArrayList<T> filtered= new ArrayList<T>();
        for (T each : getChildren()) {
            if (shouldRun(each)) {
                try {
                    filterChild(each);
                    filtered.add(each);
                } catch (NoTestsRemainException e) {
                    // don't add it
                }
            }
        }
        if (filtered.isEmpty()) {
            throw new NoTestsRemainException();
        }
    }

----------------------------------------------------------------------

>Comment By: David Saff (dsaff)
Date: 2009-11-16 11:56

Message:
Moved to github

----------------------------------------------------------------------

Comment By: Reinhold Füreder (reinholdfuerede)
Date: 2009-10-27 05:43

Message:
Moved: http://github.com/KentBeck/junit/issues/#issue/38

----------------------------------------------------------------------

Comment By: David Saff (dsaff)
Date: 2009-10-23 09:44

Message:
Can you move this bug over to github?  We'll be closing this tracker down
soon.  Thanks!

----------------------------------------------------------------------

Comment By: Reinhold Füreder (reinholdfuerede)
Date: 2009-09-29 07:20

Message:
Sorry: this bug applies to JUnit 4.7 (release)

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=115278&aid=2869568&group_id=15278