Skip to content

Conversation

@sachouche
Copy link
Contributor

Analysis -

  • Test failure have been observed with tests that were aimed to be only executed with the HSQL database
  • There were two reasons for such failures
  • a) The current infrastructure will need to instantiate a data-source for each executed test; this means that HSQL database based tests still require valid MSSQL scripts to be available (valid and not-empty)
  • b) We need skip logic for HSQL based tests called with different environments

FIX

  • a) Added database scripts to avoid data source initialization failures
  • b) Added skip logic by following the same pattern that have been utilized on other tests

@pivotal-issuemaster
Copy link

@sachouche Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@sachouche Thank you for signing the Contributor License Agreement!

Copy link
Contributor

@schauder schauder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this.

There is an easier way to disable HsqlIntegrationTests for all databases except HSQLDB:
Just add @ActiveProfiles("hsql") to the test as done in MyBatisHsqlIntegrationTests.

This should also negate the need for other database setup scripts

@sachouche sachouche force-pushed the issue/DATAJDBC-346 branch from 5f5866a to 809a111 Compare March 26, 2019 19:43
@sachouche
Copy link
Contributor Author

@schauder, thank you for the feedback! your suggested fix is definitely more elegant and easier to maintain. The only test where I had to add more logic is the "EnableJdbcAuditingHsqlIntegrationTests.java" test since the logic programmatically creates new contexts..

@sachouche
Copy link
Contributor Author

@schauder, I have implemented your requested changes; can you please review them?

Thanks!

schauder pushed a commit that referenced this pull request Apr 8, 2019
schauder added a commit that referenced this pull request Apr 8, 2019
Original pull request: #144.
@schauder
Copy link
Contributor

schauder commented Apr 8, 2019

That's merged.

Thanks.

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.

3 participants