From 09aa4fd81e07268dc8f2d4aec680f1463dd46387 Mon Sep 17 00:00:00 2001 From: Dor-bl <59066376+Dor-bl@users.noreply.github.com> Date: Sun, 29 Mar 2026 20:27:22 +0000 Subject: [PATCH 1/3] perf: optimize OptionCollector by reducing redundant ToDictionary() calls - Refactored `ParseCapabilitiesIfWindows` and `ParseCapabilitiesIfUNIX` to accept a pre-computed dictionary instead of calling `ToDictionary()` internally. - Updated `Arguments` property getter to call `ToDictionary()` exactly once and reuse the result. - Optimized `AddCapabilities` to merge options in-place, avoiding multiple dictionary allocations and object re-instantiations. - Improved efficiency of option collection by reducing O(N) operations and memory pressure. --- .../Appium/Service/Options/OptionCollector.cs | 36 ++++++------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/src/Appium.Net/Appium/Service/Options/OptionCollector.cs b/src/Appium.Net/Appium/Service/Options/OptionCollector.cs index 280ef75f..0aea23c8 100644 --- a/src/Appium.Net/Appium/Service/Options/OptionCollector.cs +++ b/src/Appium.Net/Appium/Service/Options/OptionCollector.cs @@ -49,25 +49,9 @@ public OptionCollector AddCapabilities(AppiumOptions options) } else { - IDictionary originalDictionary = this.options.ToDictionary(); IDictionary givenDictionary = options.ToDictionary(); - IDictionary result = new Dictionary(originalDictionary); foreach (var item in givenDictionary) - { - if (originalDictionary.ContainsKey(item.Key)) - { - result[item.Key] = item.Value; - } - else - { - result.Add(item.Key, item.Value); - } - } - - this.options = new AppiumOptions(); - - foreach (var item in result) { this.options.AddAdditionalAppiumOption(item.Key, item.Value); } @@ -76,14 +60,12 @@ public OptionCollector AddCapabilities(AppiumOptions options) return this; } - private string ParseCapabilitiesIfWindows() + private string ParseCapabilitiesIfWindows(IDictionary capabilitiesDictionary) { string result = string.Empty; - if (options != null) + if (capabilitiesDictionary != null) { - IDictionary capabilitiesDictionary = options.ToDictionary(); - foreach (var item in capabilitiesDictionary) { object value = item.Value; @@ -127,15 +109,15 @@ private string ParseCapabilitiesIfWindows() return "\"{" + result + "}\""; } - private string ParseCapabilitiesIfUNIX() + private string ParseCapabilitiesIfUNIX(IDictionary capabilitiesDictionary) { - if (options == null) + if (capabilitiesDictionary == null) { return string.Empty; } // Serialize to JSON and escape double quotes so they survive argument parsing - var json = JsonSerializer.Serialize(options.ToDictionary()); + var json = JsonSerializer.Serialize(capabilitiesDictionary); // Escape double quotes with backslash for shell argument var escaped = json.Replace("\"", "\\\""); return $"\"{escaped}\""; @@ -164,16 +146,18 @@ internal IList Arguments } } - if (options != null && options.ToDictionary().Count > 0) + var optionsDictionary = options?.ToDictionary(); + + if (optionsDictionary != null && optionsDictionary.Count > 0) { result.Add(CapabilitiesFlag); if (Platform.CurrentPlatform.IsPlatformType(PlatformType.Windows)) { - result.Add(ParseCapabilitiesIfWindows()); + result.Add(ParseCapabilitiesIfWindows(optionsDictionary)); } else { - result.Add(ParseCapabilitiesIfUNIX()); + result.Add(ParseCapabilitiesIfUNIX(optionsDictionary)); } } From 24914923b0fb9b5663966ad9566ee65e9249ad4f Mon Sep 17 00:00:00 2001 From: Dor-bl Date: Sun, 29 Mar 2026 22:42:48 +0200 Subject: [PATCH 2/3] fix: initialize result list with an empty array instead of List --- src/Appium.Net/Appium/Service/Options/OptionCollector.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Appium.Net/Appium/Service/Options/OptionCollector.cs b/src/Appium.Net/Appium/Service/Options/OptionCollector.cs index 0aea23c8..24425df6 100644 --- a/src/Appium.Net/Appium/Service/Options/OptionCollector.cs +++ b/src/Appium.Net/Appium/Service/Options/OptionCollector.cs @@ -130,7 +130,7 @@ internal IList Arguments { get { - List result = new List(); + List result = []; var keys = CollectedArgs.Keys; foreach (var key in keys) { From 226f2fe964fa1974eab31e61b4a260e74695921b Mon Sep 17 00:00:00 2001 From: Dor-bl Date: Sun, 29 Mar 2026 22:42:54 +0200 Subject: [PATCH 3/3] test: add AddCapabilities_MergesCapabilitiesWhenCalledMultipleTimes test for OptionCollector --- .../AppiumLocalServerLaunchingTest.cs | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/test/integration/ServerTests/AppiumLocalServerLaunchingTest.cs b/test/integration/ServerTests/AppiumLocalServerLaunchingTest.cs index 55a60878..36dc10d8 100644 --- a/test/integration/ServerTests/AppiumLocalServerLaunchingTest.cs +++ b/test/integration/ServerTests/AppiumLocalServerLaunchingTest.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Net; +using System.Reflection; using System.Text; using System.Threading; using Appium.Net.Integration.Tests.Helpers; @@ -211,6 +212,37 @@ public void CheckAbilityToStartServiceUsingCapabilities() } } + [Test] + public void AddCapabilities_MergesCapabilitiesWhenCalledMultipleTimes() + { + var firstCapabilities = new AppiumOptions(); + firstCapabilities.AddAdditionalAppiumOption(MobileCapabilityType.PlatformName, "Android"); + + var secondCapabilities = new AppiumOptions(); + secondCapabilities.AddAdditionalAppiumOption(AndroidMobileCapabilityType.AppPackage, "io.appium.android.apis"); + + var collector = new OptionCollector() + .AddCapabilities(firstCapabilities) + .AddCapabilities(secondCapabilities); + + var argumentsProperty = typeof(OptionCollector).GetProperty("Arguments", BindingFlags.Instance | BindingFlags.NonPublic); + Assert.That(argumentsProperty, Is.Not.Null, "Arguments property not found."); + + var arguments = argumentsProperty.GetValue(collector) as IList; + Assert.That(arguments, Is.Not.Null, "Arguments should not be null."); + + var capabilitiesIndex = arguments.IndexOf("--default-capabilities"); + Assert.That(capabilitiesIndex, Is.GreaterThanOrEqualTo(0), "Capabilities flag not found."); + Assert.That(arguments.Count, Is.GreaterThan(capabilitiesIndex + 1), "Capabilities value missing."); + + var capabilitiesArgument = arguments[capabilitiesIndex + 1]; + Assert.Multiple(() => + { + Assert.That(capabilitiesArgument, Does.Contain("platformName")); + Assert.That(capabilitiesArgument, Does.Contain("appPackage")); + }); + } + [Test] public void CheckAbilityToStartServiceUsingCapabilitiesAndFlags() { @@ -344,4 +376,4 @@ public void AddingInvalidNodeArgumentThrowsException(string argument) Assert.Throws(() => serviceBuilder.WithNodeArguments(argument)); } } -} \ No newline at end of file +}