Merge pull request #46 from csf-dev/44-dupe-defensive-code

Resolve #44 - Logic corrected

Author: craigfowler

CSF.Extensions.WebDriver.Tests/Factories/WebDriverCreationConfigureOptionsTests.cs

@@ -138,7 +138,7 @@ public async Task ConfigureShouldBeAbleToGetSelectedConfigWhenASelectedConfigIsN
     }
 
     [Test,AutoMoqData]
-    public async Task ConfigureShouldProvideNullSelectedConfigWhenThereAreTwoConfigsAndNoExplicitSelection([StandardTypes] IGetsWebDriverAndOptionsTypes typeProvider)
+    public async Task ConfigureShouldProvideThrowWhenThereAreTwoConfigsAndNoExplicitSelection([StandardTypes] IGetsWebDriverAndOptionsTypes typeProvider)
     {
         var options = await GetOptionsAsync(typeProvider,
 @"{
@@ -148,7 +148,7 @@ public async Task ConfigureShouldProvideNullSelectedConfigWhenThereAreTwoConfigs
     }
 }");
 
-        Assert.That(options.GetSelectedConfiguration(), Is.Null);
+        Assert.That(() => options.GetSelectedConfiguration(), Throws.InvalidOperationException);
     }
 
     [Test,AutoMoqData]

CSF.Extensions.WebDriver.Tests/Factories/WebDriverFactoryExtensionsTests.cs

@@ -0,0 +1,22 @@
+namespace CSF.Extensions.WebDriver.Factories;
+
+[TestFixture,Parallelizable]
+public class WebDriverFactoryExtensionsTests
+{
+    [Test,AutoMoqData]
+    public void GetWebDriverShouldNotThrowIfSelectedConfigIsEmptyButOnlyOneConfigPresent(ICreatesWebDriverFromOptions factory,
+                                                                                         WebDriverCreationOptions options,
+                                                                                         WebDriverAndOptions expectedResult)
+    {
+        var configCollection = new WebDriverCreationOptionsCollection();
+        configCollection.DriverConfigurations.Add("config", options);
+        WebDriverAndOptions? result = null;
+        Mock.Get(factory).Setup(x => x.GetWebDriver(options, null)).Returns(expectedResult);
+
+        Assert.Multiple(() =>
+        {
+            Assert.That(() => result = factory.GetWebDriver(configCollection), Throws.Nothing, "Does not throw exception");
+            Assert.That(result, Is.SameAs(expectedResult), "Correct expected result");
+        });
+    }
+}

CSF.Extensions.WebDriver/Factories/WebDriverCreationOptionsCollection.cs

@@ -57,14 +57,19 @@ public IDictionary<string,WebDriverCreationOptions> DriverConfigurations
         /// <see cref="DriverConfigurations"/>; that single configuration is considered to be currently selected.</description></item>
         /// </list>
         /// </remarks>
-        /// <returns>A <see cref="WebDriverCreationOptions"/>, or a <see langword="null" /> reference if both <see cref="SelectedConfiguration"/>
-        /// is <see langword="null" /> or an empty string and there is not precisely one configuration within <see cref="DriverConfigurations"/>.</returns>
+        /// <returns>A <see cref="WebDriverCreationOptions"/></returns>
         /// <exception cref="InvalidOperationException">If <see cref="SelectedConfiguration"/> is not <see langword="null" /> or empty, but
-        /// there is no item within <see cref="DriverConfigurations"/> with a key matching the selected configuration.</exception>
+        /// there is no item within <see cref="DriverConfigurations"/> with a key matching the selected configuration
+        /// or if <see cref="SelectedConfiguration"/> is either <see langword="null" /> or empty and there is not precisely one configuration
+        /// within <see cref="DriverConfigurations"/>.</exception>
         public WebDriverCreationOptions GetSelectedConfiguration()
         {
             if (string.IsNullOrEmpty(SelectedConfiguration))
-                return DriverConfigurations.Count == 1 ? DriverConfigurations.Single().Value : null;
+            {
+                return DriverConfigurations.Count == 1
+                    ? DriverConfigurations.Single().Value
+                    : throw new InvalidOperationException($"If the {nameof(SelectedConfiguration)} is not set then there must be precisely one configuration present in {nameof(DriverConfigurations)}.");
+            }
 
             if(driverConfigurations.TryGetValue(SelectedConfiguration, out var options)) return options;
             throw new InvalidOperationException($"The {nameof(SelectedConfiguration)}: '{SelectedConfiguration}' must exist as a key within the {nameof(DriverConfigurations)}.");

CSF.Extensions.WebDriver/Factories/WebDriverFactoryExtensions.cs

@@ -32,9 +32,6 @@ public static class WebDriverFactoryExtensions
         /// <exception cref="ArgumentException">
         /// If any of:
         /// <list type="bullet">
-        /// <item><description>The <paramref name="configuration"/> has a <see langword="null" /> or empty <see cref="WebDriverCreationOptionsCollection.SelectedConfiguration"/></description></item>
-        /// <item><description>The <paramref name="configuration"/> has no entry within <see cref="WebDriverCreationOptionsCollection.DriverConfigurations"/> with a key matching the
-        /// <see cref="WebDriverCreationOptionsCollection.SelectedConfiguration"/></description></item>
         /// <item><description>The selected <see cref="WebDriverCreationOptionsCollection.DriverConfigurations"/> entry is <see langword="null" /></description></item>
         /// <item><description>The <see cref="WebDriverCreationOptions.DriverType"/> of the selected <see cref="WebDriverCreationOptionsCollection.DriverConfigurations"/> is <see langword="null" /> or empty</description></item>
         /// <item><description>The <see cref="WebDriverCreationOptions.OptionsFactory"/> of the selected <see cref="WebDriverCreationOptionsCollection.DriverConfigurations"/> is <see langword="null" /></description></item>
@@ -46,18 +43,21 @@ public static class WebDriverFactoryExtensions
         /// Either <see cref="WebDriverCreationOptions.DriverType"/> or <see cref="WebDriverCreationOptions.OptionsType"/> of the
         /// selected <see cref="WebDriverCreationOptionsCollection.DriverConfigurations"/> are non-null/non-empty but no type can be found matching the specifed values.
         /// </exception>
+        /// <exception cref="InvalidOperationException">
+        /// If <see cref="WebDriverCreationOptionsCollection.SelectedConfiguration"/> is <see langword="null" /> or empty and there is not precisely one
+        /// configuration within <see cref="WebDriverCreationOptionsCollection.DriverConfigurations"/>.
+        /// Or if the <paramref name="configuration"/> does not contain a configuration item with a key matching the
+        /// <see cref="WebDriverCreationOptionsCollection.SelectedConfiguration"/>.
+        /// </exception>
         public static WebDriverAndOptions GetWebDriver(this ICreatesWebDriverFromOptions factory,
                                               WebDriverCreationOptionsCollection configuration,
                                               Action<DriverOptions> supplementaryConfiguration = null)
         {
             if (factory is null) throw new ArgumentNullException(nameof(factory));
             if (configuration is null) throw new ArgumentNullException(nameof(configuration));
-            if (string.IsNullOrEmpty(configuration.SelectedConfiguration))
-                throw new ArgumentException($"The {nameof(WebDriverCreationOptionsCollection)}.{nameof(WebDriverCreationOptionsCollection.SelectedConfiguration)} must not be null or empty.", nameof(configuration));
-            if (!configuration.DriverConfigurations.TryGetValue(configuration.SelectedConfiguration, out WebDriverCreationOptions value) || value is null)
-                throw new ArgumentException($"The {nameof(WebDriverCreationOptionsCollection)}.{nameof(WebDriverCreationOptionsCollection.DriverConfigurations)} must contain a non-null entry matching the {nameof(WebDriverCreationOptionsCollection.SelectedConfiguration)}: '{configuration.SelectedConfiguration}'. No such entry was found.", nameof(configuration));
 
-            return factory.GetWebDriver(value, supplementaryConfiguration);
+            var configItem = configuration.GetSelectedConfiguration() ?? throw new ArgumentException($"The selected web driver configuration item must not be null.", nameof(configuration));
+            return factory.GetWebDriver(configItem, supplementaryConfiguration);
         }
 
         /// <summary>