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.

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

Conversation

domharrington
Copy link
Member

@domharrington domharrington commented Apr 28, 2022

🧰 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.

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.
@domharrington domharrington changed the title Feature/cross language integration tests dotnet feature: cross language integration tests Apr 28, 2022
@domharrington domharrington changed the title feature: cross language integration tests feature: cross language integration tests (dotnet edition) Apr 28, 2022
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 domharrington merged commit 5e88624 into feature/cross-language-integration-tests Apr 29, 2022
@domharrington domharrington deleted the feature/cross-language-integration-tests-dotnet branch April 29, 2022 11:16
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant