Unit Test Loop gets executed more than once

Question

For some reason the loop inside a method, that gets executed in a unit test, gets executed more than once. And because of that I get ConcurrentModificationException. To make it short, method loops through objects, executes other method on each object, with Runnable parameter. This works just fine when application is deployed, but during unit testing, loop gets executed more than once and I get an error.

Code example:

@RunWith(JukitoRunner.class)
public class MyTest {

    @Inject
    MainService mainService;

    @Test
    public void testMain(){
        mainService.setData(mainService.getSelectedData());
    }
}

public class MainService {

    List<Data> data = new ArrayList<Data>();

    List<Field> fields = new ArrayList<Field>();

    public MainService(){
        /* this.fields is filled here*/
        data.add(/*data obj*/);
        data.add(/*data obj*/);
        data.add(/*data obj*/);
    }

    public List<Data> getSelectedData(){
        /* alghoritm to filter data */
        return data; /*returns List with 1 and 2nd data objects from this.data*/
    }
    private void deleteEl(Field field, Runnable callback){
        fields.remove(field);
        for (ListIterator<Data> i = data.listIterator(); i.hasNext();) {
            Data data = i.next();
            if(data.something()) i.remove();
        }

        if (callback != null) {
            callback.run();
        }
    }

    public void setData(List<Data> selected){
        for(Field field : fields){// checked with debug, this gets executed more than once, why?! It should run only once. ConcurrentModificationException gets thrown here.
            if(field instanceof Object){
                deleteEl(field, new Runnable(){
                    @Override
                    public void run(){
                        create(selected); //won't post create() code, since even commenting this, does not help. Error persists
                    }
                })
            }
        }
    }

}

Have You tried debugging it step by step and/or setting breakpoints to inspect what could be the cause? What error is thrown? And why is there a test case with no assertions? — PanBrambor, Aug 09 '17 at 08:10
You delete while iterating through `fields`. This is a bug not related to the test. — C-Otto, Aug 09 '17 at 08:10
`setData` iterates through `fields`, and calls `deleteEl`. `deleteEl` calls `fields.remove(field)`. You can't do that. — nbrooks, Aug 09 '17 at 08:11
I just posted bare bones of the test, only the part that is the cause, hence no assertions.... — CrazySabbath, Aug 09 '17 at 08:11
@nbrooks, that makes sence, but why Can't I reproduce this bug when application is deployed? Damn, even at production server I can't reproduce this. Only after i wrote unit test (code was not written by me and ofcourse there was no unit test..) did I run into this. — CrazySabbath, Aug 09 '17 at 08:12
I can't say why your production code is or isn't working, but the `ConcurrentModificationException` error you're seeing here is because of the code mentioned above. That's definitely a bug. If you have a different issue, with different code, feel free to post that. As an FYI, `remove(obj)` does loop through the collection as well, which you might be confusing with your outer loop. — nbrooks, Aug 09 '17 at 08:15

score 2 · Answer 1 · answered Aug 09 '17 at 08:14

2

The exception occurs because you remove a field from the fields list (first line in deleteEl method) while iterating over the fields list (for(Field field : fields).

Btw. I assume the check for (field instanceof Object) always returns true.

answered Aug 09 '17 at 08:14

Gerald Mücke

10,724
2
50
67

If `field` is null then `field instanceof Object` would return false. So it's a complicated null check here I guess... – nbrooks Aug 09 '17 at 08:17
aggreed, explicit check for `field != null` would be more comprehensible – Gerald Mücke Aug 09 '17 at 08:22
`Btw. I assume the check for (field instanceof Object) always returns true.` Wrong assumption. Anyway, problem has something to do with @nbrooks notes, though it's a bit more complicated – CrazySabbath Aug 09 '17 at 08:56
I don't get it, either its a `Field`, a subtype of `Field` or `null` ... so why don't check for null instead? – Gerald Mücke Aug 09 '17 at 09:15
Field is just an example, it doesn't relate to problem at hand, so I didn't bother to post my real code. In truth, instead of field would be a Base class, and there are several different extensions of that class, so there's a check wether Base is of a particular instance (not lang.Object obviously). – CrazySabbath Aug 09 '17 at 10:50

Unit Test Loop gets executed more than once

1 Answers1