Remove early exit from parsing function

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

@@ -32,6 +32,15 @@ public void GetDefaultWebDriverShouldReturnADriverProxyWithIdentification()
         Assert.That(() => driver.WebDriver.GetBrowserId(), Is.Not.Null);
     }
 
+    [Test]
+    public void GetWebDriverWithOptionsShouldNotThrow()
+    {
+        var services = GetServiceProvider(o => o.SelectedConfiguration = "FakeWithOptions");
+        var driverFactory = services.GetRequiredService<IGetsWebDriver>();
+        using var driver = driverFactory.GetDefaultWebDriver();
+        Assert.That(() => driver.WebDriver.GetBrowserId(), Is.Not.Null);
+    }
+
     [Test]
     public void DriverTypeNorOptionsTypeShouldBeMandatoryIfACustomFactoryTypeIsSpecified()
     {
@@ -108,4 +117,12 @@ public WebDriverAndOptions GetWebDriver(WebDriverCreationOptions options, Action
             return new(Mock.Of<IWebDriver>(), Mock.Of<DriverOptions>());
         }
     }
+
+    public class FakeOptionsUsingWebDriverFactory : ICreatesWebDriverFromOptions
+    {
+        public WebDriverAndOptions GetWebDriver(WebDriverCreationOptions options, Action<DriverOptions>? supplementaryConfiguration = null)
+        {
+            return new(Mock.Of<IWebDriver>(), options.OptionsFactory());
+        }
+    }
 }

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

@@ -106,6 +106,24 @@ public void GetWebDriverShouldCustomiseDriverOptionsWithCallbackWhenItIsSpecifie
         Assert.That(driverOptions.ToCapabilities()["Foo"], Is.EqualTo("Bar"));
     }
 
+    [Test,AutoMoqData]
+    public void GetWebDriverShouldThrowInvalidOperationExceptionIfOptionsFactoryIsUnset(
+        [StandardTypes] IGetsWebDriverAndOptionsTypes typeProvider,
+        [Frozen] IServiceProvider services,
+        WebDriverFromThirdPartyFactory sut)
+    {
+        var options = new WebDriverCreationOptions
+        {
+            DriverType = nameof(RemoteWebDriver),
+            GridUrl = "nonsense://127.0.0.1:1/nonexistent/path",
+            DriverFactoryType = typeof(FakeWebDriverFactory).AssemblyQualifiedName,
+        };
+        Mock.Get(services).Setup(x => x.GetService(typeof(FakeWebDriverFactory))).Returns(() => new FakeWebDriverFactory());
+        Mock.Get(typeProvider).Setup(x => x.GetWebDriverFactoryType(typeof(FakeWebDriverFactory).AssemblyQualifiedName)).Returns(typeof(FakeWebDriverFactory));
+
+        Assert.That(() => sut.GetWebDriver(options, null), Throws.InvalidOperationException);
+    }
+
     public class FakeWebDriverFactory : ICreatesWebDriverFromOptions
     {
         public WebDriverAndOptions GetWebDriver(WebDriverCreationOptions options, Action<DriverOptions>? supplementaryConfiguration = null)

CSF.Extensions.WebDriver.Tests/appsettings.WebDriverFactoryIntegrationTests.json

@@ -18,6 +18,12 @@
                 "OptionsType": "ChromeOptions",
                 "GridUrl": "nonsense://127.0.0.1/no-http-request/should-be-made"
             },
+            "FakeWithOptions": {
+                "DriverType": "RemoteWebDriver",
+                "OptionsType": "ChromeOptions",
+                "GridUrl": "invalid://127.0.0.1/no-http-request/should-be-made",
+                "DriverFactoryType": "CSF.Extensions.WebDriver.Factories.WebDriverFactoryIntegrationTests+FakeOptionsUsingWebDriverFactory, CSF.Extensions.WebDriver.Tests"
+            },
             "OmittedDriverAndOptionsType": {
                 "DriverFactoryType": "CSF.Extensions.WebDriver.Factories.WebDriverFactoryIntegrationTests+FakeWebDriverFactory, CSF.Extensions.WebDriver.Tests"
             },

CSF.Extensions.WebDriver/Factories/WebDriverConfigurationItemParser.cs

@@ -61,16 +61,15 @@ public WebDriverCreationOptions GetDriverConfiguration(IConfigurationSection con
         bool TryGetDriverType(WebDriverCreationOptions options, IConfigurationSection configuration, out Type driverType)
         {
             driverType = null;
-            if(options.DriverFactoryType != null)
-                return true;
 
             if(options.DriverType is null)
             {
-                logger.LogError("{ParamName} is mandatory unless {FactoryTypeKey} is specified; the configuration '{ConfigKey}' will be omitted.",
-                                nameof(WebDriverCreationOptions.DriverType),
-                                nameof(WebDriverCreationOptions.DriverFactoryType),
-                                configuration.Key);
-                return false;
+                if(options.DriverFactoryType == null)
+                    logger.LogError("{ParamName} is mandatory unless {FactoryTypeKey} is specified; the configuration '{ConfigKey}' will be omitted.",
+                                    nameof(WebDriverCreationOptions.DriverType),
+                                    nameof(WebDriverCreationOptions.DriverFactoryType),
+                                    configuration.Key);
+                return options.DriverFactoryType != null ? true : false;
             }
 
             try
@@ -80,14 +79,15 @@ bool TryGetDriverType(WebDriverCreationOptions options, IConfigurationSection co
             }
             catch(Exception e)
             {
-                logger.LogError(e,
-                                "No implementation of {WebDriverIface} could be found for the {DriverTypeProp} '{DriverType}'; the driver configuration '{ConfigKey}' will be omitted. " +
-                                "Reminder: If the driver type is not one which is shipped with Selenium then you must specify its assembly-qualified type name.",
-                                nameof(IWebDriver),
-                                nameof(WebDriverCreationOptions.DriverType),
-                                options.DriverType,
-                                configuration.Key);
-                return false;
+                if(options.DriverFactoryType == null)
+                    logger.LogError(e,
+                                    "No implementation of {WebDriverIface} could be found for the {DriverTypeProp} '{DriverType}'; the driver configuration '{ConfigKey}' will be omitted. " +
+                                    "Reminder: If the driver type is not one which is shipped with Selenium then you must specify its assembly-qualified type name.",
+                                    nameof(IWebDriver),
+                                    nameof(WebDriverCreationOptions.DriverType),
+                                    options.DriverType,
+                                    configuration.Key);
+                return options.DriverFactoryType != null ? true : false;
             }
         }
 
@@ -110,25 +110,25 @@ bool TryGetDriverType(WebDriverCreationOptions options, IConfigurationSection co
         bool TryGetOptionsType(WebDriverCreationOptions options, IConfigurationSection configuration, Type driverType, out Type optionsType)
         {
             optionsType = null;
-            if(options.DriverFactoryType != null)
-                return true;
 
             try
             {
                 optionsType = typeProvider.GetWebDriverOptionsType(driverType, options.OptionsType);                
             }
             catch(Exception e)
             {
-                logger.LogError(e,
-                                "No type deriving from {OptionsBase} could be found for the combination of {WebDriverIface} {DriverType} and {OptionsTypeProp} '{OptionsType}'; the configuration '{ConfigKey}' will be omitted. " +
-                                "See the exception details for more information.",
-                                nameof(DriverOptions),
-                                nameof(IWebDriver),
-                                driverType.Name,
-                                nameof(WebDriverCreationOptions.OptionsType),
-                                options.OptionsType,
-                                configuration.Key);
-                return false;
+                if(options.DriverFactoryType == null)
+                    logger.LogError(e,
+                                    "No type deriving from {OptionsBase} could be found for the combination of {WebDriverIface} {DriverType} and {OptionsTypeProp} '{OptionsType}'; the configuration '{ConfigKey}' will be omitted. " +
+                                    "See the exception details for more information.",
+                                    nameof(DriverOptions),
+                                    nameof(IWebDriver),
+                                    driverType?.Name,
+                                    nameof(WebDriverCreationOptions.OptionsType),
+                                    options?.OptionsType,
+                                    configuration.Key);
+
+                return options.DriverFactoryType != null ? true : false;
             }
 
             try
@@ -138,12 +138,13 @@ bool TryGetOptionsType(WebDriverCreationOptions options, IConfigurationSection c
             }
             catch(Exception e)
             {
-                logger.LogError(e,
-                                "An unexpected error occurred creating or binding to the {OptionsClass} type {OptionsType}; the configuration '{ConfigKey}' will be omitted.",
-                                nameof(DriverOptions),
-                                optionsType.FullName,
-                                configuration.Key);
-                return false;
+                if(options.DriverFactoryType == null)
+                    logger.LogError(e,
+                                    "An unexpected error occurred creating or binding to the {OptionsClass} type {OptionsType}; the configuration '{ConfigKey}' will be omitted.",
+                                    nameof(DriverOptions),
+                                    optionsType.FullName,
+                                    configuration.Key);
+                return options.DriverFactoryType != null ? true : false;
             }
         }
 

CSF.Extensions.WebDriver/Factories/WebDriverCreationOptions.cs

@@ -1,6 +1,7 @@
 using System;
 using CSF.Extensions.WebDriver.Proxies;
 using OpenQA.Selenium;
+using OpenQA.Selenium.DevTools;
 
 namespace CSF.Extensions.WebDriver.Factories
 {
@@ -16,6 +17,8 @@ namespace CSF.Extensions.WebDriver.Factories
     /// </remarks>
     public class WebDriverCreationOptions
     {
+        Func<DriverOptions> optionsFactory = GetUnsetOptionsFactory();
+
         /// <summary>
         /// Gets or sets a value indicating the concrete <see cref="Type"/> name of the class which should be used
         /// as the <see cref="IWebDriver"/> implementation.
@@ -134,15 +137,23 @@ public class WebDriverCreationOptions
         /// See the documentation for <see cref="OptionsType"/> for more information</description></item>
         /// </list>
         /// <para>
-        /// If the <see cref="DriverFactoryType"/> is in-use then this configuration property is generally unused, because the
-        /// specified factory is expected to take full control over the options creation.  It is particularly unusual to specify
+        /// If the <see cref="DriverFactoryType"/> is in-use then this configuration property is often unused, because the
+        /// specified factory usually takes full control over the options creation.  It is particularly unusual to specify
         /// this property in that scenario, because doing so would also require specifying either or both of <see cref="OptionsType"/>
         /// and <see cref="DriverType"/>, which are also typically unused when a custom factory is specified.
         /// If it is specified, then the value will be provided to the custom factory, but the factory is under no obligation to
         /// use or respect its value.
         /// </para>
+        /// <para>
+        /// Note that if neither <see cref="DriverType"/> or <see cref="OptionsType"/> are specified then this property will default
+        /// to a function which always throws <see cref="InvalidOperationException"/>, with a message indicating that it may be not be used.
+        /// </para>
         /// </remarks>
-        public Func<DriverOptions> OptionsFactory { get; set; }
+        public Func<DriverOptions> OptionsFactory
+        {
+            get => optionsFactory;
+            set => optionsFactory = value ?? GetUnsetOptionsFactory();
+        }
 
         /// <summary>
         /// An optional object which implements <see cref="ICustomizesOptions{TOptions}"/> for the corresponding <see cref="DriverOptions"/>
@@ -264,5 +275,10 @@ public class WebDriverCreationOptions
         /// </para>
         /// </remarks>
         public bool AddBrowserQuirks { get; set; } = true;
+
+        static Func<DriverOptions> GetUnsetOptionsFactory()
+        {
+            return () => throw new InvalidOperationException($"Driver options cannot be created via {nameof(OptionsFactory)}; either {nameof(DriverType)} must be set to a type which indicates a deterministic options type or {nameof(OptionsType)} must set set. If you are using a custom {nameof(DriverFactoryType)} then it may not be appropriate to create options in this way.");
+        }
     }
 }