Fix MergedContextConfiguration cache key for runtime contexts by sbrannen · Pull Request #1341 · spring-attic/spring-native

sbrannen · 2021-12-05T14:41:25Z

This is a fix for #1262.

A more detailed commit message will be crafted after peer review.

sbrannen · 2021-12-06T09:43:10Z

Web support is not working with the status quo, so I'll have to take another look.

Caused by: java.lang.ClassCastException: class org.springframework.aot.test.AotMergedContextConfiguration cannot be cast to class org.springframework.test.context.web.WebMergedContextConfiguration (org.springframework.aot.test.AotMergedContextConfiguration and org.springframework.test.context.web.WebMergedContextConfiguration are in unnamed module of loader 'app')
	at org.springframework.aot.test.boot.AotSpringBootConfigContextLoader$WebConfigurer.configure(AotSpringBootConfigContextLoader.java:135)
	at org.springframework.aot.test.boot.AotSpringBootConfigContextLoader.loadContext(AotSpringBootConfigContextLoader.java:103)
	at org.springframework.aot.test.boot.AotSpringBootConfigContextLoader.loadContext(AotSpringBootConfigContextLoader.java:50)
	at org.springframework.aot.test.AotCacheAwareContextLoaderDelegate.loadContextInternal(AotCacheAwareContextLoaderDelegate.java:62)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:124)
	... 68 common frames omitted

snicoll

As requested by @sdeleuze, here's a first review. Happy to chat if more details are needed.

...n/java/org/springframework/aot/test/context/bootstrap/generator/TestContextAotProcessor.java

...a/org/springframework/aot/test/context/bootstrap/generator/TestContextAotProcessorTests.java

spring-native/src/main/java/org/springframework/aot/test/AotContextLoader.java

spring-native/src/main/java/org/springframework/aot/test/AotMergedContextConfiguration.java

snicoll · 2021-12-06T16:50:56Z

...tive/src/test/java/org/springframework/aot/test/AotCacheAwareContextLoaderDelegateTests.java

+		MergedContextConfiguration mergedConfig = createMergedContextConfiguration(SampleTest.class);
+		assertThat(delegate.loadContext(mergedConfig)).isSameAs(this.applicationContext);
+
+		// loadContext() must actually be invoked with the original MergedContextConfiguration


It will take me a bit of time to process this but, at a quick glance, I generally feel that a "lot' of explanations like that in a test is the sign that something else should be made more explicit.

Fair enough.

I'll move the comments to actual Javadoc in the production code.

spring-native/src/main/java/org/springframework/aot/test/AotMergedContextConfiguration.java

sdeleuze · 2021-12-07T07:39:36Z

Please let me know when this PR is ready for final review and merge.

sbrannen · 2021-12-07T07:54:33Z

Please let me know when this PR is ready for final review and merge.

I'll post here when it's ready.

See spring-atticgh-1262

…mode See spring-atticgh-1262

See spring-atticgh-1262

These tests also indirectly verify the implementation of equals() and hashCode() for instances of AotMergedContextConfiguration created by AotCacheAwareContextLoaderDelegate. See spring-atticgh-1262

sbrannen · 2021-12-07T15:51:55Z

This PR is ready for a final review.

The only remaining task is an update to the Javadoc (see #1341 (comment)), but that can be done at later date if necessary.

See gh-1341

snicoll · 2021-12-07T18:09:48Z

Thanks Sam, this looks good.

sbrannen requested a review from snicoll December 5, 2021 14:41

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 5, 2021

sbrannen marked this pull request as draft December 6, 2021 09:43

sbrannen force-pushed the issues/gh-1262-AOT-MergedContextConfiguration branch from fe9b0df to dd5fdb5 Compare December 6, 2021 12:12

sbrannen marked this pull request as ready for review December 6, 2021 12:13

sbrannen force-pushed the issues/gh-1262-AOT-MergedContextConfiguration branch from dd5fdb5 to 855495b Compare December 6, 2021 12:17

snicoll reviewed Dec 6, 2021

View reviewed changes

sdeleuze added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 7, 2021

snicoll added this to the 0.11.0 milestone Dec 7, 2021

snicoll mentioned this pull request Dec 7, 2021

Fix MergedContextConfiguration cache key for runtime contexts #1262

Closed

snicoll changed the title ~~Introduce AotMergedContextConfiguration for tests in AOT mode~~ Fix MergedContextConfiguration cache key for runtime contexts Dec 7, 2021

sbrannen added 9 commits December 7, 2021 16:48

Introduce AotMergedContextConfiguration for tests in AOT mode

320df02

See spring-atticgh-1262

Reinstate @disabled integration test for context caching in AOT test …

12a2d36

…mode See spring-atticgh-1262

Revise TestContextAotProcessor according to review feedback

3f4526d

See spring-atticgh-1262

Revise TestContextAotProcessorTests according to review feedback

0f06bed

See spring-atticgh-1262

Revise AotCacheAwareContextLoaderDelegate according to review feedback

1830ad8

See spring-atticgh-1262

Reduce duplication in AotContextLoader according to review feedback

3ef445f

See spring-atticgh-1262

Rename AotContextLoader to AotTestMappings and update Javadoc

d9d03db

See spring-atticgh-1262

Introduce unit tests for AotMergedContextConfiguration

905fbba

See spring-atticgh-1262

Introduce integration tests for AotCacheAwareContextLoaderDelegate

56f3be2

These tests also indirectly verify the implementation of equals() and hashCode() for instances of AotMergedContextConfiguration created by AotCacheAwareContextLoaderDelegate. See spring-atticgh-1262

sbrannen force-pushed the issues/gh-1262-AOT-MergedContextConfiguration branch from 855495b to 56f3be2 Compare December 7, 2021 15:50

snicoll pushed a commit that referenced this pull request Dec 7, 2021

Introduce AotMergedContextConfiguration for tests in AOT mode

36dff57

See gh-1341

snicoll added a commit that referenced this pull request Dec 7, 2021

Polish "Introduce AotMergedContextConfiguration for tests in AOT mode"

4280ad5

See gh-1341

snicoll closed this in 07a4e66 Dec 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix MergedContextConfiguration cache key for runtime contexts#1341

Fix MergedContextConfiguration cache key for runtime contexts#1341
sbrannen wants to merge 9 commits intospring-attic:mainfrom
sbrannen:issues/gh-1262-AOT-MergedContextConfiguration

sbrannen commented Dec 5, 2021

Uh oh!

sbrannen commented Dec 6, 2021

Uh oh!

snicoll left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snicoll Dec 6, 2021

Uh oh!

sbrannen Dec 7, 2021

Uh oh!

Uh oh!

sdeleuze commented Dec 7, 2021

Uh oh!

sbrannen commented Dec 7, 2021 •

edited

Loading

Uh oh!

sbrannen commented Dec 7, 2021

Uh oh!

snicoll commented Dec 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Conversation

sbrannen commented Dec 5, 2021

Uh oh!

sbrannen commented Dec 6, 2021

Uh oh!

snicoll left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snicoll Dec 6, 2021

Choose a reason for hiding this comment

Uh oh!

sbrannen Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sdeleuze commented Dec 7, 2021

Uh oh!

sbrannen commented Dec 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbrannen commented Dec 7, 2021

Uh oh!

snicoll commented Dec 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

sbrannen commented Dec 7, 2021 •

edited

Loading