Skip to content

Set embedded test database as primary. #7217

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

Closed
wants to merge 1 commit into from
Closed

Set embedded test database as primary. #7217

wants to merge 1 commit into from

Conversation

gdpotter
Copy link
Contributor

@gdpotter gdpotter commented Oct 24, 2016

When there are multiple datasources present, make sure that the AutoConfigureTestDatabase annotation marks the embedded source as primary. Otherwise, it will still swap out the primary source but then the HibernateJpaAutoConfiguration won't be able to inject the DataSource because there will be 2 beans and neither of them will be marked as primary.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 24, 2016
@philwebb
Copy link
Member

I wonder if instead of always marking the embedded one as primary we should instead copy isPrimary from the original bean definition.

@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 24, 2016
@philwebb philwebb added this to the 1.4.2 milestone Oct 24, 2016
@gdpotter
Copy link
Contributor Author

I considered that but since the only time we create the embedded one is if there was only one (in which case adding primary doesn't do anything) or if there were multiple and it was the primary one. Happy to revise if you think that's better.

When there are multiple datasources present, make sure that the AutoConfigureTestDatabase annotation marks the embedded source as primary.
philwebb added a commit that referenced this pull request Oct 31, 2016
* pr/7217:
  Respect 'primary' flag when replacing databases
@philwebb
Copy link
Member

Thanks, I've reworked the PR a little to copy the flag. This should be in the next 1.4.x release.

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

Successfully merging this pull request may close these issues.

3 participants