Description

I am testing a bulk insert in a mobgoDB database, in the @BeforeEach method I instantiate the database, in the@AfterEach I delete all documents created in the puzzles-today collection.

But the last one always fails, have random result, sometimes the test passes sometimes it displays this 367 or 366 or 364 or 364.

I added @RepeatedTest(4) To make sure it doesn't vary.

The code :

Class test

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.List;

import com.github.mathieusoysal.codingame_stats.CodinGame;
import com.github.mathieusoysal.codingame_stats.puzzle.Puzzle;

import org.bson.Document;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.Test;

import io.github.mathieusoysal.util.MongoDBMockTest;

public class PuzzleDaoTest extends MongoDBMockTest {

    private PuzzlesDao puzzleDao;

    @BeforeEach
    public void setUp() throws Exception {
        super.setUp();
        puzzleDao = new PuzzlesDao(mongoClient, "CodinGame-stats");
    }

    @AfterEach
    public void tearDown() throws Exception {
        mongoClient.getDatabase("CodinGame-stats")
                .getCollection(PuzzlesDao.PUZZLES_HISTORY_COLLECTION)
                .deleteMany(new Document());
        super.tearDown();
    }

    @Test
    void testSaveAll_shouldReturnTrue_withTwoPuzzles() {
        List<Puzzle> puzzles = new CodinGame().getPuzzles().subList(0, 2);
        assertTrue(puzzleDao.saveAll(puzzles));
    }

    @Test
    void testSaveAll_shouldAugmentCollectionSize_withTwoPuzzles() {
        List<Puzzle> puzzles = new CodinGame().getPuzzles().subList(0, 2);
        puzzleDao.saveAll(puzzles);
        assertEquals(puzzles.size(), countDocuments());
    }

    @RepeatedTest(4)
    void testSaveAll_shouldAugmentCollectionSize_withAllPuzzles() {
        List<Puzzle> puzzles = new CodinGame().getPuzzles();
        puzzleDao.saveAll(puzzles);
        assertEquals(puzzles.size(), countDocuments());
    }

    private long countDocuments() {
        return mongoClient.getDatabase("CodinGame-stats")
                .getCollection(PuzzlesDao.PUZZLES_HISTORY_COLLECTION)
                .countDocuments();
    }

}

Clone the project

To easily replicate this, you can clone the repository in the dev branch

But this result is random, sometimes the test passes sometimes it displays a error.

anyone have any idea to fix this?

CodePudding user response：

The problem you have is not a test problem, but an implementation problem. Look at:

class PuzzlesDao...
    public boolean saveAll(List<Puzzle> puzzles) {
        List<WriteModel<Document>> bulkWrites = new ArrayList<>();
        GsonBuilder builder = new GsonBuilder();
        Gson gson = builder.create();
        puzzles.parallelStream()
                .forEach(puzzle -> bulkWrites.add(new InsertOneModel<>(Document.parse(gson.toJson(puzzle)))));
        BulkWriteOptions bulkWriteOptions = new BulkWriteOptions().ordered(false);
        BulkWriteResult bulkWriteResult = collection.bulkWrite(bulkWrites, bulkWriteOptions);
        return bulkWriteResult.wasAcknowledged();
    }

You are using parallelStream() without a closing operation like collect(..) or count(). The forEach code is still being executed while the bulk operation is already running. Thus the resulting behaviour is unpredictable. One heuristic is to never use any side-effects in forEach(). If you have to then using a simple for loop is the better choice in my opinion.

I recommend the following change:

public boolean saveAll(List<Puzzle> puzzles) {
    GsonBuilder builder = new GsonBuilder();
    Gson gson = builder.create();
    List<WriteModel<Document>> bulkWrites =
            puzzles.parallelStream()
                    .map(puzzle -> new InsertOneModel<>(Document.parse(gson.toJson(puzzle))))
                    .collect(Collectors.toList());
    BulkWriteOptions bulkWriteOptions = new BulkWriteOptions().ordered(false);
    BulkWriteResult bulkWriteResult = collection.bulkWrite(bulkWrites, bulkWriteOptions);
    return bulkWriteResult.wasAcknowledged();
}

I personally hardly ever use parallelStream() unless I can show that it improves performance in a worthwhile way, which it usually does not. Using just stream() gets rid of the intricacies of parallelism.