Skip to content

Commit 6f14924

Browse files
craigfowlercraigfowler
authored
Merge pull request #58 from csf-dev/57-driver-options-unset
Remove early exit from parsing function
2 parents 8cdc425 + e68af4a commit 6f14924

5 files changed

Lines changed: 94 additions & 36 deletions

File tree

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,15 @@ public void GetDefaultWebDriverShouldReturnADriverProxyWithIdentification()
3232
Assert.That(() => driver.WebDriver.GetBrowserId(), Is.Not.Null);
3333
}
3434

35+
[Test]
36+
public void GetWebDriverWithOptionsShouldNotThrow()
37+
{
38+
var services = GetServiceProvider(o => o.SelectedConfiguration = "FakeWithOptions");
39+
var driverFactory = services.GetRequiredService<IGetsWebDriver>();
40+
using var driver = driverFactory.GetDefaultWebDriver();
41+
Assert.That(() => driver.WebDriver.GetBrowserId(), Is.Not.Null);
42+
}
43+
3544
[Test]
3645
public void DriverTypeNorOptionsTypeShouldBeMandatoryIfACustomFactoryTypeIsSpecified()
3746
{
@@ -108,4 +117,12 @@ public WebDriverAndOptions GetWebDriver(WebDriverCreationOptions options, Action
108117
return new(Mock.Of<IWebDriver>(), Mock.Of<DriverOptions>());
109118
}
110119
}
120+
121+
public class FakeOptionsUsingWebDriverFactory : ICreatesWebDriverFromOptions
122+
{
123+
public WebDriverAndOptions GetWebDriver(WebDriverCreationOptions options, Action<DriverOptions>? supplementaryConfiguration = null)
124+
{
125+
return new(Mock.Of<IWebDriver>(), options.OptionsFactory());
126+
}
127+
}
111128
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,24 @@ public void GetWebDriverShouldCustomiseDriverOptionsWithCallbackWhenItIsSpecifie
106106
Assert.That(driverOptions.ToCapabilities()["Foo"], Is.EqualTo("Bar"));
107107
}
108108

109+
[Test,AutoMoqData]
110+
public void GetWebDriverShouldThrowInvalidOperationExceptionIfOptionsFactoryIsUnset(
111+
[StandardTypes] IGetsWebDriverAndOptionsTypes typeProvider,
112+
[Frozen] IServiceProvider services,
113+
WebDriverFromThirdPartyFactory sut)
114+
{
115+
var options = new WebDriverCreationOptions
116+
{
117+
DriverType = nameof(RemoteWebDriver),
118+
GridUrl = "nonsense://127.0.0.1:1/nonexistent/path",
119+
DriverFactoryType = typeof(FakeWebDriverFactory).AssemblyQualifiedName,
120+
};
121+
Mock.Get(services).Setup(x => x.GetService(typeof(FakeWebDriverFactory))).Returns(() => new FakeWebDriverFactory());
122+
Mock.Get(typeProvider).Setup(x => x.GetWebDriverFactoryType(typeof(FakeWebDriverFactory).AssemblyQualifiedName)).Returns(typeof(FakeWebDriverFactory));
123+
124+
Assert.That(() => sut.GetWebDriver(options, null), Throws.InvalidOperationException);
125+
}
126+
109127
public class FakeWebDriverFactory : ICreatesWebDriverFromOptions
110128
{
111129
public WebDriverAndOptions GetWebDriver(WebDriverCreationOptions options, Action<DriverOptions>? supplementaryConfiguration = null)

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@
1818
"OptionsType": "ChromeOptions",
1919
"GridUrl": "nonsense://127.0.0.1/no-http-request/should-be-made"
2020
},
21+
"FakeWithOptions": {
22+
"DriverType": "RemoteWebDriver",
23+
"OptionsType": "ChromeOptions",
24+
"GridUrl": "invalid://127.0.0.1/no-http-request/should-be-made",
25+
"DriverFactoryType": "CSF.Extensions.WebDriver.Factories.WebDriverFactoryIntegrationTests+FakeOptionsUsingWebDriverFactory, CSF.Extensions.WebDriver.Tests"
26+
},
2127
"OmittedDriverAndOptionsType": {
2228
"DriverFactoryType": "CSF.Extensions.WebDriver.Factories.WebDriverFactoryIntegrationTests+FakeWebDriverFactory, CSF.Extensions.WebDriver.Tests"
2329
},

‎CSF.Extensions.WebDriver/Factories/WebDriverConfigurationItemParser.cs‎

Lines changed: 34 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -61,16 +61,15 @@ public WebDriverCreationOptions GetDriverConfiguration(IConfigurationSection con
6161
bool TryGetDriverType(WebDriverCreationOptions options, IConfigurationSection configuration, out Type driverType)
6262
{
6363
driverType = null;
64-
if(options.DriverFactoryType != null)
65-
return true;
6664

6765
if(options.DriverType is null)
6866
{
69-
logger.LogError("{ParamName} is mandatory unless {FactoryTypeKey} is specified; the configuration '{ConfigKey}' will be omitted.",
70-
nameof(WebDriverCreationOptions.DriverType),
71-
nameof(WebDriverCreationOptions.DriverFactoryType),
72-
configuration.Key);
73-
return false;
67+
if(options.DriverFactoryType == null)
68+
logger.LogError("{ParamName} is mandatory unless {FactoryTypeKey} is specified; the configuration '{ConfigKey}' will be omitted.",
69+
nameof(WebDriverCreationOptions.DriverType),
70+
nameof(WebDriverCreationOptions.DriverFactoryType),
71+
configuration.Key);
72+
return options.DriverFactoryType != null ? true : false;
7473
}
7574

7675
try
@@ -80,14 +79,15 @@ bool TryGetDriverType(WebDriverCreationOptions options, IConfigurationSection co
8079
}
8180
catch(Exception e)
8281
{
83-
logger.LogError(e,
84-
"No implementation of {WebDriverIface} could be found for the {DriverTypeProp} '{DriverType}'; the driver configuration '{ConfigKey}' will be omitted. " +
85-
"Reminder: If the driver type is not one which is shipped with Selenium then you must specify its assembly-qualified type name.",
86-
nameof(IWebDriver),
87-
nameof(WebDriverCreationOptions.DriverType),
88-
options.DriverType,
89-
configuration.Key);
90-
return false;
82+
if(options.DriverFactoryType == null)
83+
logger.LogError(e,
84+
"No implementation of {WebDriverIface} could be found for the {DriverTypeProp} '{DriverType}'; the driver configuration '{ConfigKey}' will be omitted. " +
85+
"Reminder: If the driver type is not one which is shipped with Selenium then you must specify its assembly-qualified type name.",
86+
nameof(IWebDriver),
87+
nameof(WebDriverCreationOptions.DriverType),
88+
options.DriverType,
89+
configuration.Key);
90+
return options.DriverFactoryType != null ? true : false;
9191
}
9292
}
9393

@@ -110,25 +110,25 @@ bool TryGetDriverType(WebDriverCreationOptions options, IConfigurationSection co
110110
bool TryGetOptionsType(WebDriverCreationOptions options, IConfigurationSection configuration, Type driverType, out Type optionsType)
111111
{
112112
optionsType = null;
113-
if(options.DriverFactoryType != null)
114-
return true;
115113

116114
try
117115
{
118116
optionsType = typeProvider.GetWebDriverOptionsType(driverType, options.OptionsType);
119117
}
120118
catch(Exception e)
121119
{
122-
logger.LogError(e,
123-
"No type deriving from {OptionsBase} could be found for the combination of {WebDriverIface} {DriverType} and {OptionsTypeProp} '{OptionsType}'; the configuration '{ConfigKey}' will be omitted. " +
124-
"See the exception details for more information.",
125-
nameof(DriverOptions),
126-
nameof(IWebDriver),
127-
driverType.Name,
128-
nameof(WebDriverCreationOptions.OptionsType),
129-
options.OptionsType,
130-
configuration.Key);
131-
return false;
120+
if(options.DriverFactoryType == null)
121+
logger.LogError(e,
122+
"No type deriving from {OptionsBase} could be found for the combination of {WebDriverIface} {DriverType} and {OptionsTypeProp} '{OptionsType}'; the configuration '{ConfigKey}' will be omitted. " +
123+
"See the exception details for more information.",
124+
nameof(DriverOptions),
125+
nameof(IWebDriver),
126+
driverType?.Name,
127+
nameof(WebDriverCreationOptions.OptionsType),
128+
options?.OptionsType,
129+
configuration.Key);
130+
131+
return options.DriverFactoryType != null ? true : false;
132132
}
133133

134134
try
@@ -138,12 +138,13 @@ bool TryGetOptionsType(WebDriverCreationOptions options, IConfigurationSection c
138138
}
139139
catch(Exception e)
140140
{
141-
logger.LogError(e,
142-
"An unexpected error occurred creating or binding to the {OptionsClass} type {OptionsType}; the configuration '{ConfigKey}' will be omitted.",
143-
nameof(DriverOptions),
144-
optionsType.FullName,
145-
configuration.Key);
146-
return false;
141+
if(options.DriverFactoryType == null)
142+
logger.LogError(e,
143+
"An unexpected error occurred creating or binding to the {OptionsClass} type {OptionsType}; the configuration '{ConfigKey}' will be omitted.",
144+
nameof(DriverOptions),
145+
optionsType.FullName,
146+
configuration.Key);
147+
return options.DriverFactoryType != null ? true : false;
147148
}
148149
}
149150

‎CSF.Extensions.WebDriver/Factories/WebDriverCreationOptions.cs‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using CSF.Extensions.WebDriver.Proxies;
33
using OpenQA.Selenium;
4+
using OpenQA.Selenium.DevTools;
45

56
namespace CSF.Extensions.WebDriver.Factories
67
{
@@ -16,6 +17,8 @@ namespace CSF.Extensions.WebDriver.Factories
1617
/// </remarks>
1718
public class WebDriverCreationOptions
1819
{
20+
Func<DriverOptions> optionsFactory = GetUnsetOptionsFactory();
21+
1922
/// <summary>
2023
/// Gets or sets a value indicating the concrete <see cref="Type"/> name of the class which should be used
2124
/// as the <see cref="IWebDriver"/> implementation.
@@ -134,15 +137,23 @@ public class WebDriverCreationOptions
134137
/// See the documentation for <see cref="OptionsType"/> for more information</description></item>
135138
/// </list>
136139
/// <para>
137-
/// If the <see cref="DriverFactoryType"/> is in-use then this configuration property is generally unused, because the
138-
/// specified factory is expected to take full control over the options creation. It is particularly unusual to specify
140+
/// If the <see cref="DriverFactoryType"/> is in-use then this configuration property is often unused, because the
141+
/// specified factory usually takes full control over the options creation. It is particularly unusual to specify
139142
/// this property in that scenario, because doing so would also require specifying either or both of <see cref="OptionsType"/>
140143
/// and <see cref="DriverType"/>, which are also typically unused when a custom factory is specified.
141144
/// If it is specified, then the value will be provided to the custom factory, but the factory is under no obligation to
142145
/// use or respect its value.
143146
/// </para>
147+
/// <para>
148+
/// Note that if neither <see cref="DriverType"/> or <see cref="OptionsType"/> are specified then this property will default
149+
/// to a function which always throws <see cref="InvalidOperationException"/>, with a message indicating that it may be not be used.
150+
/// </para>
144151
/// </remarks>
145-
public Func<DriverOptions> OptionsFactory { get; set; }
152+
public Func<DriverOptions> OptionsFactory
153+
{
154+
get => optionsFactory;
155+
set => optionsFactory = value ?? GetUnsetOptionsFactory();
156+
}
146157

147158
/// <summary>
148159
/// An optional object which implements <see cref="ICustomizesOptions{TOptions}"/> for the corresponding <see cref="DriverOptions"/>
@@ -264,5 +275,10 @@ public class WebDriverCreationOptions
264275
/// </para>
265276
/// </remarks>
266277
public bool AddBrowserQuirks { get; set; } = true;
278+
279+
static Func<DriverOptions> GetUnsetOptionsFactory()
280+
{
281+
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.");
282+
}
267283
}
268284
}

0 commit comments

Comments
 (0)