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.

Sign up for GitHub

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

Jump to bottom

Add SQL backend for E2E Tests #3047

Merged
merged 8 commits into from
Feb 26, 2025
Merged

Add SQL backend for E2E Tests #3047

merged 8 commits into from
Feb 26, 2025
+169 −157

Conversation

andystaples
Copy link
Contributor

Adds the MSSQL storage backend for the new end-to-end tests. Runs a subset of the tests against the new backend.

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation PR is ready to merge and referenced in pending_docs.md
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)
  • My changes do not require any extra work to be leveraged by OutOfProc SDKs
    • Otherwise: That work is being tracked here: #issue_or_pr_in_each_sdk
  • My changes do not change the version of the WebJobs.Extensions.DurableTask package
    • Otherwise: major or minor version updates are reflected in /src/Worker.Extensions.DurableTask/AssemblyInfo.cs
  • My changes do not add EventIds to our EventSource logs
    • Otherwise: Ensure the EventIds are within the supported range in our existing Windows infrastructure. You may validate this with a deployed app's telemetry. You may also extend the range by completing a PR such as this one.
  • My changes should be added to v2.x branch.
    • Otherwise: This change applies exclusively to WebJobs.Extensions.DurableTask v3.x. It will be retained only in the dev and main branches and will not be merged into the v2.x branch.

Sorry, something went wrong.

@andystaples
Add SQL backend for E2E Tests
Loading
Loading status checks…
705900d
@andystaples
Run azurite on mssql tests for functions
Loading
Loading status checks…
1d0b6ff
@andystaples
Randomly generate MSSQL password
Loading
Loading status checks…
4a3a794
@andystaples
Runsettings file improvements
95ac7e3
@andystaples
Pipeline set env var approach
Loading
Loading status checks…
2ea23d2
@andystaples
Update issue in comments
Loading
Loading status checks…
011e551
@andystaples @nytian
Update test/e2e/Tests/Tests/ErrorHandlingTests.cs
Loading
Loading status checks…
6097eaa 
Co-authored-by: Naiyuan Tian <110135109+nytian@users.noreply.github.com>
nytian
nytian approved these changes Feb 26, 2025
View reviewed changes
Copy link
Collaborator

@nytian nytian 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 the PR. I noticed the CI passed. I just left a few questions

test/e2e/Tests/build-e2e-test.ps1
@@ -28,7 +25,6 @@ $ProjectBaseDirectory = "$PSScriptRoot\..\..\..\"
$ProjectTemporaryPath = Join-Path ([System.IO.Path]::GetTempPath()) "DurableTaskExtensionE2ETests"
New-Item -Path $ProjectTemporaryPath -ItemType Directory -ErrorAction SilentlyContinue
$WebJobsExtensionProjectDirectory = Join-Path $ProjectBaseDirectory "src\WebJobs.Extensions.DurableTask"
$WorkerExtensionProjectDirectory = Join-Path $ProjectBaseDirectory "src\Worker.Extensions.DurableTask"
Copy link
Collaborator

@nytian nytian Feb 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

why do me skip worker extensions? Same question as another comment on E2ETest.yml

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 worker extension is referenced as a project reference in the functionapp csproj, and will be built and referenced in the app build stage. However, due to local NuGet caching strategies, we need to build/pack/delete/redeploy the WebJobs extension manually to ensure that during the build of the generated Extensions.csproj for the functionapp, it gets any local changes that may differ from nuget.org or previous local builds.

test/e2e/Tests/build-e2e-test.ps1

[Switch]
$SkipCoreTools,

[Switch]
$SkipBuildOnPack
$SkipBuild
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not quite sure what this SkipBuild is used for? Seems like we never pass this argument,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was using it locally to test the docker logic further down the scriptfile without needing to rebuild these projects every time. I left it in as it may come in handy for those who want to redeploy docker/azurite locally without interfering with the prior build.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment with this information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

test/e2e/Tests/test.runsettings
@@ -0,0 +1,15 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is this file for? Is it for customers when they want to locally debug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! By default, running the tests in the Visual Studio test explorer will test only Azure Storage. This file provides a way to configure the environment variables necessary for MSSQL testing locally

.github/workflows/E2ETest.yml
@@ -14,8 +14,10 @@ on:
- 'test/e2e/**'

jobs:
build:
e2e-azurestorage:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if I understand it right --based on the workflow title, this e2e tests should be for .NET isolated apps, not for .NET in-proc apps. But why we only build the pkg webjobs.extensions.durabletask, and remove worker.extensions.durabletask ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment on build-e2e-test.ps1 - the Worker extension gets built as a project reference, but the WebJobs extension will fall back on possibly invalid cached versions in the local NuGet cache unless we explicitly build it in this way

bachuv
bachuv approved these changes Feb 26, 2025
View reviewed changes
test/e2e/Tests/build-e2e-test.ps1

[Switch]
$SkipCoreTools,

[Switch]
$SkipBuildOnPack
$SkipBuild
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment with this information?

@andystaples
Comment in build-e2e-test.ps1
Loading
Loading status checks…
284397a
@andystaples andystaples merged commit f0818f8 into dev Feb 26, 2025
7 checks passed
@andystaples andystaples deleted the andystaples/add-mssql-backend-to-e2e-tests branch February 26, 2025 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bachuv bachuv

@nytian nytian

@cgillum cgillum

@sophiatev sophiatev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@andystaples @bachuv @nytian