Skip to content

Conversation

@nmummau
Copy link
Contributor

@nmummau nmummau commented Oct 24, 2025

Purpose

The purpose of this PR is to enrich the SQL Server support.
Here is a video that walks through the changes and does a demo: https://youtu.be/PJV-JueZUmk

Overview

  • Created 2 new database projects
    • Eventuous.SqlServer.Database.csproj
    • Eventuous.Tests.SqlServer.Database.csproj

Each of these database projects have 0 impact on code that Eventuous deploys.

Eventuous.SqlServer.Database.csproj

full path: src/SqlServer/src/Eventuous.SqlServer.Database/Eventuous.SqlServer.Database.csproj

  • Does not effect how the Eventuous deploys the SQL Server database schema
  • Store database definition, matching the migration files defined here: Scripts
  • The reason those migration Scripts are not "good enough" is because they do not allow us to harness the power of SQL Server database projects:
    • Static code analysis
    • DACPAC enabling automated testing withing docker
  • Within this project are also a few things to enable tSQLt testing
    • Dockerfile.db-from-dacpac deploys the database to a containerized database with the DACPAC using sqlpackage. This allows to initialize a ephemeral SQL Server database for automated testing

Static code analysis

This is a mechanism that allows us to identify code smells and other possible issues with the SQL definitions. Some of those are too opinionated for Eventuous, such as the need for a transaction. Those false positive things can be suppressed in the StaticCodeAnalysis.SuppressMessages.xml file. Right now there are 4 things being suppressed. As a follow-up PR to this one, I'll create Issues to address those.

DACPAC

If you are not familiar, a .dacpac file is a build artifact as a result of building a database project. That .dacpac file can then be used to "Publish" to a database. Pairing this with docker allows to create temporary containerized databases for various purposes, but what I'm adding here is focused on tSQLt testing.

Eventuous.Tests.SqlServer.Database.csproj

full path: ‎src/SqlServer/test/Eventuous.Tests.SqlServer.Database/Eventuous.Tests.SqlServer.Database.csproj

  • Contains tSQLt tests. Learn more about the testing framework here
  • Dockerfile.eventuous-db-tsqlt-runner‎ creates and runs the database tests on the ephemeral SQL Server database I mentioned above.

- new database projects
    - ‎src/SqlServer/src/Eventuous.SqlServer.Database/Eventuous.SqlServer.Database.csproj
        - store database definition
        - static code analysis
        - no relevance to how the database is deployed, other than for tSQLt testing purposes
        - Dockerfile.db-from-dacpac deploys the database to a containerized database with sqlpackage. This is used for tSQLt testing.
    - ‎src/SqlServer/test/Eventuous.Tests.SqlServer.Database/Eventuous.Tests.SqlServer.Database.csproj
        - store database tSQLt tests
        - Dockerfile.eventuous-db-tsqlt-runner‎ deploys and runs the database tests
- minor SQL Server procedure formatting
    - check_stream add semicolon
    - read_stream_sub remove indenty
@alexeyzimarev
Copy link
Contributor

How would it interact with the primary project? Those script are included in the database project but remained in the main project?

@alexeyzimarev alexeyzimarev requested a review from Copilot October 27, 2025 12:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces SQL Server database projects to enable static code analysis and automated testing using the tSQLt framework. The implementation includes database schema definitions via DACPAC deployment and a Docker-based testing infrastructure for running unit tests against SQL Server stored procedures and database objects.

  • Creates database project structure with schema definitions matching existing migration scripts
  • Implements tSQLt-based unit tests for database procedures and constraints
  • Establishes Docker infrastructure for ephemeral database testing environments

Reviewed Changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/SqlServer/src/Eventuous.SqlServer.Database/* Database schema definitions (tables, procedures, types) mirroring existing migration scripts
src/SqlServer/src/Eventuous.SqlServer.Database/docker/* Docker configuration for deploying database from DACPAC
src/SqlServer/test/Eventuous.Tests.SqlServer.Database/* tSQLt test suite covering database procedures and constraints
src/SqlServer/test/Eventuous.Tests.SqlServer.Database/docker/* Docker test runner infrastructure for automated tSQLt execution
Directory.Packages.props Package references for database project tooling
nuget.config NuGet configuration for package restoration
.gitattributes Ensures shell scripts use LF line endings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

SQL="EXEC tSQLt.Run '$1'"
;;
*)

Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

[nitpick] Extra blank line 43 between the case pattern and the heredoc statement should be removed to improve code readability.

Suggested change

Copilot uses AI. Check for mistakes.

USER root

ADD https://aka.ms/sqlpackage-linux /tmp/sqlpackage.zip
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

The sqlpackage zip is downloaded twice (lines 30 and 43). The first ADD instruction appears to be unused and should be removed to avoid unnecessary downloads and confusion.

Suggested change
ADD https://aka.ms/sqlpackage-linux /tmp/sqlpackage.zip

Copilot uses AI. Check for mistakes.
END;
GO

CREATE PROCEDURE Messages.[Test StreamPosition is invalid json ExpectExcepton]
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'ExpectExcepton' to 'ExpectException'.

Copilot uses AI. Check for mistakes.
END;
GO

CREATE PROCEDURE Messages.[Test CK_eventuous_Messages_JsonData violation ExpectExcepton]
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'ExpectExcepton' to 'ExpectException'.

Copilot uses AI. Check for mistakes.
END;
GO

CREATE PROCEDURE Messages.[Test CK_eventuous_Messages_JsonMetadata violation ExpectExcepton]
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'ExpectExcepton' to 'ExpectException'.

Copilot uses AI. Check for mistakes.
END;
GO

CREATE PROCEDURE Messages.[Test UQ_eventuous_Messages_StreamId_StreamPosition violoation ExpectExcepton]
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'violoation' to 'violation' and 'ExpectExcepton' to 'ExpectException'.

Suggested change
CREATE PROCEDURE Messages.[Test UQ_eventuous_Messages_StreamId_StreamPosition violoation ExpectExcepton]
CREATE PROCEDURE Messages.[Test UQ_eventuous_Messages_StreamId_StreamPosition violation ExpectException]

Copilot uses AI. Check for mistakes.
END;
GO

CREATE PROCEDURE Messages.[Test UQ_eventuous_Messages_StreamId_MessageId violation ExpectExcepton]
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'ExpectExcepton' to 'ExpectException'.

Copilot uses AI. Check for mistakes.
END;
GO

CREATE PROCEDURE Checkpoints.[Test UQ_eventuous_Checkpoints_Id violation ExpectExcepton]
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'ExpectExcepton' to 'ExpectException'.

Suggested change
CREATE PROCEDURE Checkpoints.[Test UQ_eventuous_Checkpoints_Id violation ExpectExcepton]
CREATE PROCEDURE Checkpoints.[Test UQ_eventuous_Checkpoints_Id violation ExpectException]

Copilot uses AI. Check for mistakes.
END;
GO

CREATE PROCEDURE Streams.[Test CK_eventuous_Streams_Version violation ExpectExcepton]
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'ExpectExcepton' to 'ExpectException'.

Suggested change
CREATE PROCEDURE Streams.[Test CK_eventuous_Streams_Version violation ExpectExcepton]
CREATE PROCEDURE Streams.[Test CK_eventuous_Streams_Version violation ExpectException]

Copilot uses AI. Check for mistakes.
END;
GO

CREATE PROCEDURE check_stream.[Test stream exists wrong version ExpectExcepton]
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'ExpectExcepton' to 'ExpectException'.

Suggested change
CREATE PROCEDURE check_stream.[Test stream exists wrong version ExpectExcepton]
CREATE PROCEDURE check_stream.[Test stream exists wrong version ExpectException]

Copilot uses AI. Check for mistakes.
@nmummau
Copy link
Contributor Author

nmummau commented Oct 27, 2025

How would it interact with the primary project? Those script are included in the database project but remained in the main project?

There is no formal interaction between the main project and this new database project. If scripts are changed, we'd need to change them in both places for accuracy.

The main project holds the scripts needed for migrating the database.
The new database project is needed for tsqlt testing and static code analysis. Violations of tests or static code analysis can be a build failure to alert us of breaking schema changes.

@alexeyzimarev
Copy link
Contributor

Maybe use project file links to share the scripts between those two projects?

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.

2 participants