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

Ignore duplicate configuration metadata for cache key in the TestContext framework #25800

Closed
3 tasks done
dfit99 opened this issue Sep 22, 2020 · 5 comments
Closed
3 tasks done
Assignees
Labels
in: test Issues in the test module type: enhancement A general enhancement
Milestone

Comments

@dfit99
Copy link

dfit99 commented Sep 22, 2020

This affects 4.3

When I have a base class like:

@ContextConfiguration(classes = A.class)
class Base extends AbstractTestNGSpringContextTests {}

and a child class:

@ContextConfiguration(classes = A.class)
class Child extends Base {}

I noticed Spring creates separate ApplicationContext for both test classes.

This is not the expected behavior since according to the docs:

An ApplicationContext can be uniquely identified by the combination of configuration parameters that are used to load it

So by the definition outlined in the docs, these two test classes should being using the same ApplicationContext .

When I enabled cache debugging I noticed this being logged:

Storing ApplicationContext in cache under key [[MergedContextConfiguration@4f190018 testClass = Base, locations = '{}', classes = '{class A}

when trying to retrieve the ApplicationContext for the base class.

Storing ApplicationContext in cache under key [[MergedContextConfiguration@4f190017 testClass = Child, locations = '{}', classes = '{class A, class A}

when trying to retrieve the ApplicationContext for the child class.

I expect the cache key to be the same here, why is it different?

Deliverables

Ignore duplicate test configuration annotation (for the following list of annotations) when a given test class declares the exact same annotation (or equivalent merged annotation) as the next level above the current test class in the inheritance class hierarchy or enclosing class hierarchy.

Effectively, we want to remove exact duplication resulting from copy-n-paste errors before building the MergedContextConfiguration for a given test class.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 22, 2020
@dfit99 dfit99 changed the title Chaining ContextConfiguration classes of Incorrect cache key being created to retrieve ApplicationContext in Tests Sep 22, 2020
@sbrannen
Copy link
Member

I expect the cache key to be the same here, why is it different?

They are different, because Base only declares A.class; whereas, Child declares A.class and also inherits A.class from Base.

If you don't want this to happen, you have two options.

  1. Don't redeclare @ContextConfiguration(classes = A.class) on Child, because it's inherited from Base.
  2. Instruct Spring not to inherit the configuration classes via @ContextConfiguration(classes = A.class, inheritLocations = false) on Child.

Out of curiosity, why do you redeclare the exact same configuration on Child?

@sbrannen sbrannen added in: test Issues in the test module status: waiting-for-feedback We need additional information before we can continue labels Sep 23, 2020
@sbrannen
Copy link
Member

In any case, I'll investigate whether it's feasible to ignore such duplicate configuration metadata within a test class hierarchy for Spring Framework 5.3.

@sbrannen sbrannen self-assigned this Sep 23, 2020
@sbrannen sbrannen added status: pending-design-work Needs design work before any code can be developed type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 23, 2020
@sbrannen sbrannen changed the title Incorrect cache key being created to retrieve ApplicationContext in Tests Ignore duplicate configuration metadata for cache key in the TestContext framework Sep 23, 2020
@sbrannen sbrannen added this to the 5.3 RC2 milestone Sep 23, 2020
@dfit99
Copy link
Author

dfit99 commented Sep 23, 2020

Hi Sam,

Thank you for the quick response.

As to answer your question, we had no good reason (just dumb copying and pasting) to use the same ContextConfiguration in the child class.

However, we also didn't expect it to behave this way either.

Thank you again for looking into this issue!

@sbrannen sbrannen removed the status: waiting-for-feedback We need additional information before we can continue label Sep 23, 2020
@sbrannen sbrannen modified the milestones: 5.3 RC2, 5.3 GA Oct 13, 2020
@sbrannen
Copy link
Member

I've added a "Deliverables" section to this issue's description to define and limit the scope of this issue.

@sbrannen sbrannen removed the status: pending-design-work Needs design work before any code can be developed label Oct 24, 2020
sbrannen added a commit to sbrannen/spring-framework that referenced this issue Oct 24, 2020
This commit ignores duplicate configuration metadata when generating
the ApplicationContext cache key (i.e., MergedContextConfiguration) in
the Spring TestContext Framework. This performed for the following
annotations.

- @ContextConfiguration
- @activeprofiles
- @TestPropertySource

In addition, this commit reinstates validation of the rules for
repeated @TestPropertySource annotations that was removed when support
for @NestedTestConfiguration was introduced.

Closes spring-projectsgh-25800
@sbrannen
Copy link
Member

Current work on this issue can be viewed in the following feature branch.

master...sbrannen:issues/gh-25800

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants