Skip to content

Enable database upgrade testing in the CI - #331

Merged
andystaples merged 2 commits into
microsoft:mainfrom
usemam:enable-upgrade-tests-in-ci
Sep 25, 2026
Merged

andystaples merged 2 commits into
microsoft:mainfrom
usemam:enable-upgrade-tests-in-ci

Conversation

@usemam

Copy link
Copy Markdown
Contributor

Implements #152

ValidateUpgradedOrchestrations has been skipped since it was added, because RESTORE DATABASE is executed by SQL Server rather than by the test process: the backup file has to exist on the server's file system, which in CI is inside the mssql-server container and not the runner's working directory.

Stage the backup into the container with docker cp before restoring, falling back to the local path when SQL Server is installed locally. The container is discovered automatically, and DTFX_TEST_SQL_CONTAINER overrides the name (or disables the behavior with "none").

The v1.0.0 backup was taken on Windows, so the data and log file paths recorded in it can't be honored by a Linux container. Relocate both files to the target server's default data directory during the restore.

Also fix the test itself, which had rotted while skipped:

  • Expect schema-1.6.0.sql in the log assertions. An upgrade from 1.0.0 executes every newer schema script, and the list had not been updated since 1.2.0.
  • Add [Collection("Integration")], which every other integration test class already has. Without it, DROP/RESTORE DATABASE runs concurrently with DatabaseManagement's CREATE DATABASE and they contend on model.
  • Raise the orchestration waits to 60s, since instances resume against a freshly restored database with a cold plan cache.
  • Stop the task hub worker when finished, rather than leaving it polling the restored database for the lifetime of the test process.

ValidateUpgradedOrchestrations has been skipped since it was added, because
RESTORE DATABASE is executed by SQL Server rather than by the test process: the
backup file has to exist on the server's file system, which in CI is inside the
mssql-server container and not the runner's working directory.

Stage the backup into the container with docker cp before restoring, falling
back to the local path when SQL Server is installed locally. The container is
discovered automatically, and DTFX_TEST_SQL_CONTAINER overrides the name (or
disables the behavior with "none").

The v1.0.0 backup was taken on Windows, so the data and log file paths recorded
in it can't be honored by a Linux container. Relocate both files to the target
server's default data directory during the restore.

Also fix the test itself, which had rotted while skipped:

* Expect schema-1.6.0.sql in the log assertions. An upgrade from 1.0.0 executes
  every newer schema script, and the list had not been updated since 1.2.0.
* Add [Collection("Integration")], which every other integration test class
  already has. Without it, DROP/RESTORE DATABASE runs concurrently with
  DatabaseManagement's CREATE DATABASE and they contend on model.
* Raise the orchestration waits to 60s, since instances resume against a
  freshly restored database with a cold plan cache.
* Stop the task hub worker when finished, rather than leaving it polling the
  restored database for the lifetime of the test process.
@usemam

Copy link
Copy Markdown
Contributor Author

Chris Gillum (@cgillum) could please take a look? Thank you.

@cgillum

Copy link
Copy Markdown
Member

Pinging Tomer Rosenthal (@torosent) and andystaples to help take a look at this PR.

@andystaples andystaples left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks

@andystaples

Copy link
Copy Markdown
Contributor

Documenting, not blocking:

  1. Local SQL Server cannot read the extracted backup — UpgradeTests.cs:111. Moving extraction to Path.GetTempPath() places the backup in the user’s private temp directory. A default SQL Server service account lacks access, so RESTORE FILELIST fails with OS error 5: Access is denied. Use a SQL-readable staging location or explicitly grant the required access.
  2. Default multi-framework execution races against the same database — UpgradeTests.cs:45. The net8.0 and net10.0 test hosts share the database and backup paths; [Collection("Integration")] does not serialize across hosts. I reproduced one framework upgrading the database before the other checked its original version. Configure TestTfmsInParallel=false in the test project—not only CI—or isolate resources per host.

@andystaples
andystaples merged commit a4e9983 into microsoft:main Sep 25, 2026
2 checks passed
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