ptupitsyn commented on a change in pull request #9628:
URL: https://github.com/apache/ignite/pull/9628#discussion_r765091305



##########
File path: 
modules/platforms/dotnet/Apache.Ignite.Core.Tests/Client/Services/ServicesClientTest.cs
##########
@@ -585,4 +594,26 @@ private static IServices ServerServices
             get { return Ignition.GetIgnite().GetServices(); }
         }
     }
+
+    /// <summary>
+    /// Tests for <see cref="IServicesClient"/>.
+    /// </summary>
+    public class ServicesClientTestBinaryArrays : ServicesClientTest
+    {
+        /// <summary>Setup UseTypedArray flag value.</summary>
+        [TestFixtureSetUp]
+        public override void FixtureSetUp()
+        {
+            TestUtils.UseBinaryArray = true;

Review comment:
       Instead of using global mutable state in an unrelated class, I suggest 
to pass `useBinaryArray` as a constructor parameter like this:
   
   ```
   public class ServicesClientTestBinaryArrays : ServicesClientTest
   {
       public ServicesClientTestBinaryArrays() : base(useBinaryArray: true) {}
   }
   ```

##########
File path: 
modules/platforms/dotnet/Apache.Ignite.Core/Impl/Services/ServiceProxyInvoker.cs
##########
@@ -102,6 +102,12 @@ internal static class ServiceProxyInvoker
                         methodName, svcType, argsLength));
 
             // 2) There is more than 1 method with specified name - resolve 
with argument types.
+            methods = methods.Where(m => AreMethodArgsCompatible(arguments, 
m.GetParameters(), true)).ToArray();

Review comment:
       I tried reverting all changes in this file, and all tests pass. Please 
double check - either we need more tests, or those changes are unnecessary.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to