So basically this entire PR is changing 1 method call and a lot of tests to make certain it works as expected.

The PR needs a bit of clean up before we can merge it.

I see that we have some tests which are testing that stack overflow errors are thrown. I do not believe that we should have such tests. It is enough to have a test which fails without the fixes and it is green after we fix it.

Looking at the tests again, the tests in the cycles package can be removed.

I also see that we are modifying some existing tests such as the NumberMapperContext and removing certain things from the NumberMapperWithContext. I think that we should not remove nor update existing things and only add new fresh tests. If something is failing with the new changes then the changes are most likely not right.

The test added here is actually the correct one. Did some clean-up so that this becomes better visible.

It looks much cleaner now @Zegveld, thanks.

I played around with this and I can actually recommend simplifying it a bit more. Looking at the generated code we actually have 2 methods that will be called:

• NumberMapperContext#getInstance
• NumberMapperContext#visitNumber

If we change the test case to look something like:

    @ProcessorTest
    void numberUpdateMappingWithContextCallsCache() {
        Number target = new Number();
        NumberMapperWithContext.INSTANCE.integerToNumber( 2342, target );

        try {
            assertThat( NumberMapperContext.getVisited() ).containsExactly( target );
            NumberMapperContext.clearVisited();
            NumberMapperContext.putCache( target );
            NumberMapperWithContext.INSTANCE.integerToNumber( 25342, target );
            assertThat( NumberMapperContext.getVisited() ).isEmpty();
        }
        finally {
            NumberMapperContext.clearCache();
            NumberMapperContext.clearVisited();
        }
    }

WDYT @Zegveld?

@filiphr that test fails, because the putCache object ends up in a different hash-bucket then the object used for retrieval during getCache.

I did change the test somewhat and removed the cache checking. Because I feel that is what you were aiming for.

I had a typo in it @Zegveld. The second call needs to be done with 2342. That way the correct thing is done in the map

I had a typo in it @Zegveld. The second call needs to be done with 2342. That way the correct thing is done in the map

indeed without the typo it passes. The other thing I do not like about that test that it doesn't fit the given, when, then structure. Tests become harder to read when diverting from that structure, that's why I implemented the same situation using two tests.

I moved the clean-up-code into an @AfterEach method, because there was a chance that when one test fails for whatever reason another test could also fail. Even though that test would not fail when run alone. For example the caching method fails, instead the visit method is called. The next test that checks the visited items would also find the items from the previous test and then also fail.

Feel free to adjust it if you see more room for improvement at the tests, @filiphr