Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#46 implement support for nested tests in JUnit 5 #50

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ArneLimburg
Copy link

No description provided.

@dadrus
Copy link
Owner

dadrus commented Dec 2, 2019

Hi @ArneLimburg. Thank you for the MR. I'm glad to accept it if you add an integration test showing the functionality.

@ArneLimburg
Copy link
Author

Providing an implementation test to your code base is not that easy, because everything (even the JUnit 5 integration) is tested with JUnit 4.
Which approach should I take to write the integration test?
Shall I provide a new subproject? Or change the JUnit version of the JUnit 5 extension to JUnit 5 and provide a test then?

@dadrus
Copy link
Owner

dadrus commented Dec 2, 2019

I thought about something like (to b come part of jpa-unit-jpa2.1-hibernate-integration-test):

@ExtendWith(JpaUnit.class)
public class NestedJunit5Test {
    @PersistenceContext(unitName = "my-test-unit")
    private EntityManager entityManager;

    @Nested
    @DisplayName("Given an empty test table")
    class GivenAnEmptyTestTable {

        @Test
        @DisplayName("Then the size sould be zero")
        void thenTheSizeShouldBeZero() {
            final List<Depositor> list = entityManager
                    .createQuery("select d from Depositor d", Depositor.class)
                    .getResultList();
            assertEquals(list.size(), 0);
        }

        @Nested
        @DisplayName("When a row is inserted")
        class WhenARowIsInserted {

            @BeforeEach
            @Transactional(TransactionMode.COMMIT)
            void setUp() {
                final Depositor depositor = new Depositor("Max", "Doe");
                entityManager.persist(depositor);
            }

            @Test
            @DisplayName("Then the size should be one")
            void thenTheSizeShouldBeOne() {
                final List<Depositor> list = entityManager
                        .createQuery("select d from Depositor d", Depositor.class)
                        .getResultList();
                assertEquals(list.size(), 0);
            }
        }
    }
}

@dadrus
Copy link
Owner

dadrus commented Dec 2, 2019

Providing an implementation test to your code base is not that easy, because everything (even the JUnit 5 integration) is tested with JUnit 4.

I know. When first JUnit5 integration was implemented, JUnit5 was not released. It was still Beta I think. Updating of all these tests and a couple of other improvements/simplifications is still in my TODO list even the project doesn't look like it is maintained

@dadrus
Copy link
Owner

dadrus commented Dec 2, 2019

If you wonder why I paste a code here and not do it my myself, I've already created a local branch half a year ago where I just added the tests without any implementation to support that. Due to lack of time I didn't work on that. I'll push it, so you can see whether your implementation does provide the support I thought about.

@dadrus
Copy link
Owner

dadrus commented Dec 2, 2019

@ArneLimburg
Copy link
Author

I'll take a look at it

@dadrus
Copy link
Owner

dadrus commented Dec 2, 2019

please pull again. I've forgotten to commit the changes.

@ArneLimburg
Copy link
Author

OK, the nested test works, but there seems to be a problem with datasets or transaction. I'll take a look into it later.

@ArneLimburg
Copy link
Author

JUnit 5 introduces a new lifecycle callback "postProcessTestInstance", which does not really match the old jpa-unit code. My patch matches all to DecoratorExecutor#processBefore, which will potentially lead to multiple injections of the same EntityManager in nested situations.
To fix this, a deeper refactoring is necessary that introduces such method in DecoratorExecutor and somehow handles the old cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants