search

minvws / nl-covid19-notification-app-backend

Server-side code for CoronaMelder.
European Union Public License 1.2
77 stars 26 forks source link

Concern : In the unit-tests, run WireMock.Net on a random port instead of fixed port #23

Closed StefH closed 4 years ago

StefH commented 4 years ago

Concern

I see that WireMock.Net is used to unit-test the TheIdentityHubService in the TheIdentityHubServiceTests.cs file.

However a fixed port is used: https://github.com/minvws/nl-covid19-notification-app-backend/blob/601afa149dd631593589b383266e6b4185ab2524/Components.Tests/Icc/TheIdentityHubServiceTests.cs#L35

This can cause issues when running these unit-tests on a build-server, there is not 100% guarantee that this port will be free on the OS.

My suggestion would be to start the WireMock.Net without a fixed port. Just like:

var server = WireMockServer.Start();

// Getting the random port on which the WireMock.Net server is running, can be done like:
var port = server.Ports[0];

// Or you can also get the full URL where the WireMock.Net server is running with:
var url = server.Urls[0];

However this means that setting up the TheIdentityHubOptions should be done in each unit-test because the port is not fixed anymore.

Another suggestion: I see that in every test the WireMock.Net server is started and stopped, in that case using a local variable var server = instead of defining and reusing a private class variable like private WireMockServer _Server; would be a better option.

Another idea would be to start the WireMock.Net server once during start from the test-class and reset + add a new mapping in each test. This can be a bit quicker when running the tests.

hiddehs commented 4 years ago

Thanks for the suggestions @StefH ! We will try to implement them in the near future, as they will surely improve the stability of the fresh TheIdenityHubServiceTests.

hiddehs commented 4 years ago

Suggestions implemented in latest commits