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

Added End-To-End Test for Datasource Hot-Reload #2453

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+39 −1
Open

Added End-To-End Test for Datasource Hot-Reload #2453

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ae8b0e0
Added end2end test for changes in datasource
Nov 7, 2024
a154845
Changes from comments
Nov 7, 2024
f0fb712
Comment changes
Nov 12, 2024
e95037b
Changes
Nov 12, 2024
573ffa7
Changes
Nov 15, 2024
a85f8cd
Changes
Nov 15, 2024
8dfaa5c
Changes
Nov 15, 2024
995762b
Fixed Errors
Nov 15, 2024
62642ed
Changes
Nov 15, 2024
9098cd5
Fix Errors
Nov 15, 2024
4f141ea
Changes
Nov 15, 2024
b17b25f
Fix Errors
Nov 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion src/Service.Tests/Configuration/HotReload/ConfigurationHotReloadTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class ConfigurationHotReloadTests

private static void GenerateConfigFile(
DatabaseType databaseType = DatabaseType.MSSQL,
string sessionContext = "true",
string connectionString = "",
string restPath = "rest",
string restEnabled = "true",
Expand All @@ -61,7 +62,7 @@ private static void GenerateConfigFile(
""data-source"": {
""database-type"": """ + databaseType + @""",
""options"": {
""set-session-context"": true
""set-session-context"": " + sessionContext + @"
},
""connection-string"": """ + connectionString + @"""
},
Expand Down Expand Up @@ -299,4 +300,41 @@ public async Task HotReloadConfigRuntimeGQLEnabledEndToEndTest()
// Assert
Assert.AreEqual(HttpStatusCode.NotFound, gQLResult.StatusCode);
}

/// <summary>
/// Hot reload the configuration file by saving a new database type and connection string.
/// Validate that the response from the server is correct when making a new request after
/// the change in database type.
/// </summary>
[TestCategory(MSSQL_ENVIRONMENT)]
[TestMethod]
public async Task HotReloadConfigDataSourceEndToEndTest()
{
// Arrange
RuntimeConfig previousRuntimeConfig = _configProvider.GetConfig();
MsSqlOptions previousSessionContext = previousRuntimeConfig.DataSource.GetTypedOptions<MsSqlOptions>();

// String has additions that are not in original connection string
string expectedConnectionString = $"{ConfigurationTests.GetConnectionStringFromEnvironmentConfig(TestCategory.MSSQL).Replace("\\", "\\\\")}" + "Trusted_Connection=True;";

// Act
GenerateConfigFile(
RubenCerna2079 marked this conversation as resolved.
Show resolved Hide resolved
sessionContext: "false",
connectionString: expectedConnectionString);
System.Threading.Thread.Sleep(3000);
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of adding a timer here, why dont we check for the existence of the new file, and wait until the new file is generated? Adding timer to the test makes it flaky.

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 file is already created beforehand and we are only updating the file.
We use a timer in order to ensure that the server has enough time to hot-reload before moving on with the rest of the testing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why you are using timer, but what if the update doesnt happen within 3 sec? can we instead check for the condition you expect to be true after the expiry of the timer instead of using a timer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I remember correctly, DAB is not able to work on any requests until hot-reload is finished. If that is the case, by having a request with await then the test should wait until the request and hot-reload are completed.
(Please correct me if I am wrong on that)

Copy link
Contributor

Choose a reason for hiding this comment

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

@RubenCerna2079 , what happens if we don't add a sleep, does it fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abhishekkumams, yes, since it does not give enough time for the hot-reload to take place

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created an event that triggers when hot reload is completed.
This will be used in the tests to make them wait until hot reload finishes, before continuing with the tests.


RuntimeConfig updatedRuntimeConfig = _configProvider.GetConfig();
RubenCerna2079 marked this conversation as resolved.
Show resolved Hide resolved
MsSqlOptions actualSessionContext = updatedRuntimeConfig.DataSource.GetTypedOptions<MsSqlOptions>();
JsonElement reloadGQLContents = await GraphQLRequestExecutor.PostGraphQLRequestAsync(
_testClient,
_configProvider,
GQL_QUERY_NAME,
GQL_QUERY);

// Assert
// If the assert succeeds it implies that the connection string is hot-reloadable
Assert.AreNotEqual(previousSessionContext, actualSessionContext);
Assert.AreEqual(false, actualSessionContext.SetSessionContext);
SqlTestHelper.PerformTestEqualJsonStrings(_bookDBOContents, reloadGQLContents.GetProperty("items").ToString());
Copy link
Contributor

Choose a reason for hiding this comment

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

_bookDBOContents has values from MSSQL, currently all the DBs have the same values, so we don't know if it actually picked the value from MSSQL or the updated DB. Would you be able to add one row to each db with different values which we can use for more robust testing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also Based on how our pipelines work, we do not support cross database access in single pipeline. But you can add another test which i mentioned in this issue itself.

}
}