-
Notifications
You must be signed in to change notification settings - Fork 27
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
feature: cross language integration tests (dotnet edition) #435
Merged
domharrington
merged 16 commits into
feature/cross-language-integration-tests
from
feature/cross-language-integration-tests-dotnet
Apr 29, 2022
Merged
feature: cross language integration tests (dotnet edition) #435
domharrington
merged 16 commits into
feature/cross-language-integration-tests
from
feature/cross-language-integration-tests-dotnet
Apr 29, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Generated using: ``` dotnet new web --no-https ```
Still outstanding: - make the sdk respect `METRICS_SERVER` env variable - try and get `readme__apiKey` better as an env variable - wire it up to an integration test
This is slightly confusing since if they actually wanted to set this up themselves then they would probably use appsettings.json instead. This is fine for a demo app though, and will mean it works from the integration test layer 🤞
I installed the C# vs code plugin and it did this 🤷♂️
… the env Use UriBuilder to construct a URI that always has the correct path on the end
… block Right now if metrics is down, this will cause our user's server to stop responding to requests. This is probably what you'd want in the vast majority of cases - since it's actually kinda rare you'd wanna fire and forget an HTTP request.
…cross-language-integration-tests-dotnet
When using child_process.exec, the process could not reliably be killed when the test ended, which meant the docker container never closed. This caused timeouts on the github action: https://github.com/readmeio/metrics-sdks/runs/6215430863?check_suite_focus=true
There's way too many inconsistencies between even these two web servers to do jest snapshot testing, so opting to be much more explicit about assertions for the time being. Would rather get something up and passing than not running these tests going forward.
Added to docker-compose and a new dotnet.yml to be run on gh actions
This failed on github cos it's using a real linux env: https://github.com/readmeio/metrics-sdks/runs/6226930070?check_suite_focus=true Lets see if this does the trick 🤞
domharrington
added a commit
that referenced
this pull request
Apr 29, 2022
* feat: very first pass at spinning up a mock metrics server to write tests The theory is that we'll use this, pass in the command to spin up the child server, and provide the METRICS_SERVER as an env variable that each of the sdks can look for. Once we can pass that in we can make HTTP level assertions about what the SDKs can and can't do! Lets try and write a test first against the node server and see if we can make it work 🤞 * test: add a first integration test via the new testing method Now i'm gunna try and put this into a dockerfile like httpsnippet, then I can try and add another language 🤞 * test: add docker-compose file for running node/express integration tests * test: try and get new integration tests working on gh actions * test: attempting to get integration tests working on actions * refactor: attempt to bubble up errors better * refactor: toString potential errors before rejecting * test(integration): build the node module before running tests * test(integration): try adding it as a dependent job * test(integration): remove integration test from `npm test` It's gunna be run separately via another action, so don't do it twice. * test(integration): fix incorrect eslint-config reference * test(integration): fix eslint package reference * test(integration): linty linty * test(integration): attempt to get it working * test(integration): ignore lint error in express example This dependency will be there when we run the integration tests, but it won't be when the linter gets run because the TS files haven't been built * test(integration): turning off a lint rule * feat(integration/node): move more setup to Dockerfile instead of gh action * feature: cross language integration tests (dotnet edition) (#435) * test(integration/dotnet): add super simple dotnet 6.0 test server Generated using: ``` dotnet new web --no-https ``` * test(integration/dotnet): add very basic .net 6.0 app for metrics Still outstanding: - make the sdk respect `METRICS_SERVER` env variable - try and get `readme__apiKey` better as an env variable - wire it up to an integration test * test(integration/dotnet): use regular environment variable name This is slightly confusing since if they actually wanted to set this up themselves then they would probably use appsettings.json instead. This is fine for a demo app though, and will mean it works from the integration test layer 🤞 * test(integration/node): update tests to work with new port number stuff * refactor(dotnet): stylistic and whitespace changes I installed the C# vs code plugin and it did this 🤷♂️ * refactor(dotnet): fix warning to do with null variable in test app * refactor(integration/dotnet): allow the metrics server to be set from the env Use UriBuilder to construct a URI that always has the correct path on the end * refactor(dotnet): remove the await from the metrics call so it doesnt block Right now if metrics is down, this will cause our user's server to stop responding to requests. This is probably what you'd want in the vast majority of cases - since it's actually kinda rare you'd wanna fire and forget an HTTP request. * chore(integration/dotnet): make host header lookup case insensitive * test(integration/node): fix up child process usage to use spawn When using child_process.exec, the process could not reliably be killed when the test ended, which meant the docker container never closed. This caused timeouts on the github action: https://github.com/readmeio/metrics-sdks/runs/6215430863?check_suite_focus=true * test(integration/node): this no longer needs to be executable * test(integration): remove the snapshot test for now There's way too many inconsistencies between even these two web servers to do jest snapshot testing, so opting to be much more explicit about assertions for the time being. Would rather get something up and passing than not running these tests going forward. * test(integration/dotnet): add docker file for .net test Added to docker-compose and a new dotnet.yml to be run on gh actions * refactor: rename incorrect naming of job in gh actions * test(integration/dotnet): ugh, stupid case insensitive macOS This failed on github cos it's using a real linux env: https://github.com/readmeio/metrics-sdks/runs/6226930070?check_suite_focus=true Lets see if this does the trick 🤞
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🧰 Changes
Added a super simple .net6.0 example server that mimics the functionality exactly of the express server over in #417.
You can see them side by side here:
https://github.com/readmeio/metrics-sdks/pull/417/files#diff-71803c36ab07f59d758bdcc768d226319cbdc8f57e36ed6b1604eb01dccc826d=
https://github.com/readmeio/metrics-sdks/pull/435/files#diff-fb722b815bdb167f5bfd27f02ad7652a632b8ee9bd03f502d2376bcab20eca31=
🧬 QA & Testing
Provide as much information as you can on how to test what you've done.