-
Notifications
You must be signed in to change notification settings - Fork 17
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
test: introduce BDD e2e tests #124
Conversation
Codecov Report
@@ Coverage Diff @@
## main #124 +/- ##
=======================================
Coverage 90.95% 90.95%
=======================================
Files 27 27
Lines 553 553
Branches 37 37
=======================================
Hits 503 503
Misses 37 37
Partials 13 13 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Signed-off-by: odubajDT <[email protected]>
34a5b79
to
c275445
Compare
Awesome work @odubajDT ! Very good to have this in the dotnet SDK! It's a real confidence booster. I will review. |
|
||
jobs: | ||
build-test: | ||
runs-on: windows-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@odubajDT I'm OK with consolidating jobs, but we need to run a build on windows-latest
to ensure the integrity of the net462
build.
Also, if possible (and easy) it would be good to run the e2e tests there too. If that's not possible that's OK though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, will fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, so in the current state the following tests are ran:
linux: unit tests with .Net 6 and .Net 7 + E2E tests
windows: unit tests with .Net 6, .Net 7 and .Net 4.6.2
Running e2e tests on windows might be a little bit problematic due to that container operations are only supported on Linux runners. Therefore we are unable to start the flagd container. I guess it is ok when the e2e tests are ran only on linux.
Hope this meets the requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was worried because I didn't see test commands specifically targeting the bin\Release\net462\
folder, but it seems like they are being picked up automatically in the windows build:
Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
D:\a\dotnet-sdk\dotnet-sdk\test\OpenFeature.Tests\bin\Release\net6.0\OpenFeature.Tests.dll
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.4.3+1b45f5407b (64-bit .NET 6.0.16)
[xUnit.net 00:00:07.30] Discovering: OpenFeature.Tests
[xUnit.net 00:00:07.54] Discovered: OpenFeature.Tests
[xUnit.net 00:00:07.54] Starting: OpenFeature.Tests
So I think we're good!
test/OpenFeature.IntegrationTests/Steps/EvaluationStepDefinitions.cs
Outdated
Show resolved
Hide resolved
test/OpenFeature.IntegrationTests/Steps/EvaluationStepDefinitions.cs
Outdated
Show resolved
Hide resolved
test/OpenFeature.IntegrationTests/Steps/EvaluationStepDefinitions.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! The only thing blocking my approval is this.
Signed-off-by: odubajDT <[email protected]>
Signed-off-by: odubajDT <[email protected]>
Signed-off-by: odubajDT <[email protected]>
Signed-off-by: odubajDT <[email protected]>
Signed-off-by: odubajDT <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. This is great to finally have. Thanks @odubajDT !
This PR
Related Issues
Fixes #111