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

feat: automated test isolation #952

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

feat: automated test isolation #952

wants to merge 3 commits into from

Conversation

BobdenOs
Copy link
Contributor

Concept

The basic concept of this change is to provide test isolation for all the cds-dbs databases automatically. Up till now only the cds-dbs repository has been able to run tests in parallel while using persistent database services. Therefor many stakeholders have setup their own custom credential handling. This change is aiming to make all custom test isolation solutions obsolete.

To make this solution viable for all CAP applications the following criteria have to be considered:

  1. deploy only when required
  2. allow for parallel test executions
  3. allow for parallel test runs

test run isolation

The current implementation comes with support for Travis and Github Actions by using the automatic environment variables to create test run isolations given the generic name database. Additionally for local development the default operating system user name is used as database isolation. This allows for using shared database instances when running local instances of the database are not available. If none of these stable isolation names are found a default fallback name is used. This comes with the benefit that deployment reductions are functional, but might come with side effects when multiple users are relying on the fallback database isolation.

Minimal deployment

The test isolation logic relies upon the project sources to determine what needs to be deployed and what deployments can be re used. In most applications the source is defined as *. Therefor the test isolation combines this with the cds.root to determine unique source definition.

For all tests a shared read only isolation is created which all tests will initially use. Until a write operation is detected at this point the operations are paused and the isolation is switched to a test specific isolation. When switching to a test unique isolation the database is checked for any available previously deployed isolations. If no previously deployed isolations are available a new isolation is created and deployed. After the test has finished the isolation is reset to its original deployment state and marked as available for any follow up write tests.

By assuming all tests are read only tests at the start it allows the test to avoid triggering deployment as much as possible. Even when all tests running in parallel are write tests. As long as one of them finishes before another test starts writing they are able to share the same deployment. Where if the tests are assumed to be writing tests they will all always trigger their own deployment at the start. This approach results in a maximum number of deployments equal to `number of parallel tests

@@ -3,15 +3,183 @@ const ConnectionPool = require('./generic-pool')
const infer = require('../infer')
const cds = require('@sap/cds')

const databaseModel = cds.linked({
definitions: {
schemas: new cds.entity({
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a more distinct name, like cds_test_schemas to avoid clashes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The model is inside the database isolation. It actually clashes with SYS.SCHEMAS in HANA, but as this is isolated it doesn't matter.

  • SYS
    • Database 1 <- database model
      • Tenant 1 <- application model
      • ...
    • ...

cds.on('shutdown', () => this.disconnect())
if (Object.getOwnPropertyDescriptor(cds, 'test') || this.options.isolate) {

Choose a reason for hiding this comment

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

Isn't cds.test always available? So we would also run thus code for productive server start,,

Choose a reason for hiding this comment

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

should such test DB setup not be a module outside of the productive code?

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