[
https://issues.apache.org/jira/browse/TINKERPOP-2838?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17655947#comment-17655947
]
ASF GitHub Bot commented on TINKERPOP-2838:
-------------------------------------------
FlorianHockmann commented on code in PR #1923:
URL: https://github.com/apache/tinkerpop/pull/1923#discussion_r1061581137
##########
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientBehaviorIntegrationTests.cs:
##########
@@ -84,5 +84,37 @@ await
gremlinClient.SubmitWithSingleResultAsync<Vertex>(RequestMessage.Build("1"
Assert.NotNull(response2);
Assert.Equal(1, gremlinClient.NrConnections);
}
+
+ [Fact]
+ public async Task ShouldIncludeUserAgentInHandshakeRequest()
+ {
+ var sessionId = Guid.NewGuid().ToString();
Review Comment:
(nitpick) Is the `sessionId` really needed for this test?
##########
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientBehaviorIntegrationTests.cs:
##########
@@ -84,5 +84,37 @@ await
gremlinClient.SubmitWithSingleResultAsync<Vertex>(RequestMessage.Build("1"
Assert.NotNull(response2);
Assert.Equal(1, gremlinClient.NrConnections);
}
+
+ [Fact]
+ public async Task ShouldIncludeUserAgentInHandshakeRequest()
+ {
+ var sessionId = Guid.NewGuid().ToString();
+ var poolSettings = new ConnectionPoolSettings {PoolSize = 1};
+
+ var gremlinServer = new GremlinServer(TestHost, Settings.Port);
+ var gremlinClient = new GremlinClient(gremlinServer,
messageSerializer: Serializer,
Review Comment:
Please add a `using` here to ensure that the `gremlinClient` will be
disposed at the end of this test. The same applies to the other tests in this
class.
##########
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientBehaviorIntegrationTests.cs:
##########
@@ -84,5 +84,37 @@ await
gremlinClient.SubmitWithSingleResultAsync<Vertex>(RequestMessage.Build("1"
Assert.NotNull(response2);
Assert.Equal(1, gremlinClient.NrConnections);
}
+
+ [Fact]
+ public async Task ShouldIncludeUserAgentInHandshakeRequest()
+ {
+ var sessionId = Guid.NewGuid().ToString();
+ var poolSettings = new ConnectionPoolSettings {PoolSize = 1};
Review Comment:
(nitpick) Wouldn't this also work with the default settings? I wouldn't
specify any settings in tests if they aren't needed for the specific test case
to keep the tests as simple as possible.
##########
gremlin-dotnet/src/Gremlin.Net/Process/Utils.cs:
##########
@@ -32,8 +32,11 @@ namespace Gremlin.Net.Process
/// <summary>
/// Contains useful methods that can be reused across the project.
/// </summary>
- internal static class Utils
+ public static class Utils
Review Comment:
I think we should not make such a class public only to use it in tests as it
only includes methods that are intended to be used by the driver internally.
You can instead allow `Gremlin.Net.IntegrationTest` to access internals of
`Gremlin.Net` if you add it
[here](https://github.com/apache/tinkerpop/blob/7f7d3a485c7f100f98047b71672a0c2c9ab855b4/gremlin-dotnet/src/Gremlin.Net/Properties/AssemblyInfo.cs#L26)
just like we did for `Gremlin.Net.UnitTest` already. This unfortunately
requires to also sign the `Gremlin.Net.IntegrationTest` assembly which can be
done by adding [these 3
lines](https://github.com/apache/tinkerpop/blob/master/gremlin-dotnet/test/Gremlin.Net.UnitTest/Gremlin.Net.UnitTest.csproj#L6-L8)
to its csproj file.
(I can help you with this if you need any help.)
> Add UserAgent GLV Tests
> -----------------------
>
> Key: TINKERPOP-2838
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2838
> Project: TinkerPop
> Issue Type: Improvement
> Components: driver, server
> Affects Versions: 3.5.4
> Reporter: Cole Greer
> Priority: Minor
>
> User agents were added to all GLV connection requests with
> [TINKERPOP-2480|https://issues.apache.org/jira/browse/TINKERPOP-2480]. The
> Java driver has tests added which utilize SimpleSocketServer to capture and
> echo back the user agent.
> [TINKERPOP-2819|https://issues.apache.org/jira/browse/TINKERPOP-2819] will
> bring SimpleSocketServer to all GLV's. Once this is complete additional tests
> mirroring the Java driver tests should be added to all GLV's to ensure the
> user agent is being sent and received correctly, and that the
> enableUserAgentOnConnect configuration is working correctly.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
