Skip to content

Commit

Permalink
Report binding problems to binding registry and show them as errors d…
Browse files Browse the repository at this point in the history
…uring test execution (SpecFlowOSS#2663)

* Refactor BindingSourceProcessor to collect binding errors

* Fix BindingSourceProcessorStub

* Add tests & fixes

* collect type load errors

* Extend changelog

* Replace tuple with a class on the IBindingRegistry interface
  • Loading branch information
gasparnagy authored and Code-Grump committed Mar 29, 2023
1 parent 76463e5 commit 99ce6a5
Show file tree
Hide file tree
Showing 20 changed files with 368 additions and 155 deletions.
20 changes: 20 additions & 0 deletions TechTalk.SpecFlow/Bindings/BindingError.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
namespace TechTalk.SpecFlow.Bindings;

public class BindingError
{
public BindingErrorType Type { get; }
public string Message { get; }

public BindingError(BindingErrorType type, string message)
{
Type = type;
Message = message;
}
}

public enum BindingErrorType
{
TypeLoadError,
BindingError,
StepDefinitionError,
}
48 changes: 34 additions & 14 deletions TechTalk.SpecFlow/Bindings/BindingRegistry.cs
Original file line number Diff line number Diff line change
@@ -1,30 +1,34 @@
using System;
using System.Collections.Generic;
using System.Linq;

namespace TechTalk.SpecFlow.Bindings
{
public class BindingRegistry : IBindingRegistry
{
private readonly List<IStepDefinitionBinding> stepDefinitions = new List<IStepDefinitionBinding>();
private readonly List<IStepArgumentTransformationBinding> stepArgumentTransformations = new List<IStepArgumentTransformationBinding>();
private readonly Dictionary<HookType, List<IHookBinding>> hooks = new Dictionary<HookType, List<IHookBinding>>();
private readonly List<IStepDefinitionBinding> _stepDefinitions = new();
private readonly List<IStepArgumentTransformationBinding> _stepArgumentTransformations = new();
private readonly Dictionary<HookType, List<IHookBinding>> _hooks = new();
private readonly List<BindingError> _genericBindingErrors = new();

public bool Ready { get; set; }

public bool IsValid => !GetErrorMessages().Any();

public IEnumerable<IStepDefinitionBinding> GetStepDefinitions()
{
return stepDefinitions;
return _stepDefinitions;
}

public IEnumerable<IStepDefinitionBinding> GetConsideredStepDefinitions(StepDefinitionType stepDefinitionType, string stepText)
{
//TODO: later optimize to return step definitions that has a chance to match to stepText
return stepDefinitions.Where(sd => sd.StepDefinitionType == stepDefinitionType);
return _stepDefinitions.Where(sd => sd.StepDefinitionType == stepDefinitionType);
}

public virtual IEnumerable<IHookBinding> GetHooks()
{
return hooks.Values.SelectMany(hookList => hookList);
return _hooks.Values.SelectMany(hookList => hookList);
}

public virtual IEnumerable<IHookBinding> GetHooks(HookType bindingEvent)
Expand All @@ -34,28 +38,39 @@ public virtual IEnumerable<IHookBinding> GetHooks(HookType bindingEvent)

private IEnumerable<IHookBinding> GetHookList(HookType bindingEvent)
{
if (hooks.TryGetValue(bindingEvent, out var list))
if (_hooks.TryGetValue(bindingEvent, out var list))
return list;

return Enumerable.Empty<IHookBinding>();
}

public virtual IEnumerable<IStepArgumentTransformationBinding> GetStepTransformations()
{
return stepArgumentTransformations;
}

return _stepArgumentTransformations;
}

public IEnumerable<BindingError> GetErrorMessages()
{
foreach (var genericBindingError in _genericBindingErrors)
yield return genericBindingError;

foreach (var stepDefinitionBinding in _stepDefinitions.Where(sd => !sd.IsValid))
{
yield return new BindingError(BindingErrorType.StepDefinitionError, stepDefinitionBinding.ErrorMessage);
}
}

public virtual void RegisterStepDefinitionBinding(IStepDefinitionBinding stepDefinitionBinding)
{
stepDefinitions.Add(stepDefinitionBinding);
_stepDefinitions.Add(stepDefinitionBinding);
}

private List<IHookBinding> GetHookListForRegister(HookType bindingEvent)
{
if (!hooks.TryGetValue(bindingEvent, out var list))
if (!_hooks.TryGetValue(bindingEvent, out var list))
{
list = new List<IHookBinding>();
hooks.Add(bindingEvent, list);
_hooks.Add(bindingEvent, list);
}

return list;
Expand All @@ -71,7 +86,12 @@ public virtual void RegisterHookBinding(IHookBinding hookBinding)

public virtual void RegisterStepArgumentTransformationBinding(IStepArgumentTransformationBinding stepArgumentTransformationBinding)
{
stepArgumentTransformations.Add(stepArgumentTransformationBinding);
_stepArgumentTransformations.Add(stepArgumentTransformationBinding);
}

public void RegisterGenericBindingError(BindingError error)
{
_genericBindingErrors.Add(error);
}
}
}
2 changes: 2 additions & 0 deletions TechTalk.SpecFlow/Bindings/Discovery/BindingSourceMethod.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,7 @@ public class BindingSourceMethod
public bool IsStatic { get; set; }

public BindingSourceAttribute[] Attributes { get; set; }

public override string ToString() => BindingMethod.ToString();
}
}
154 changes: 101 additions & 53 deletions TechTalk.SpecFlow/Bindings/Discovery/BindingSourceProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,17 @@ namespace TechTalk.SpecFlow.Bindings.Discovery
{
public abstract class BindingSourceProcessor : IBindingSourceProcessor
{
public static string BINDING_ATTR = typeof(BindingAttribute).FullName;
public static string BindingAttributeFullName = typeof(BindingAttribute).FullName;

private readonly IBindingFactory bindingFactory;
private readonly IBindingFactory _bindingFactory;

private BindingSourceType currentBindingSourceType = null;
private BindingScope[] typeScopes = null;
private List<IStepDefinitionBindingBuilder> _stepDefinitionBindingBuilders = new();
private BindingSourceType _currentBindingSourceType = null;
private BindingScope[] _typeScopes = null;
private readonly List<IStepDefinitionBindingBuilder> _stepDefinitionBindingBuilders = new();

protected BindingSourceProcessor(IBindingFactory bindingFactory)
{
this.bindingFactory = bindingFactory;
_bindingFactory = bindingFactory;
}

public bool CanProcessTypeAttribute(string attributeTypeName)
Expand All @@ -33,7 +33,7 @@ public bool CanProcessMethodAttribute(string attributeTypeName)

private static bool IsPotentialBindingClass(IEnumerable<string> attributeTypeNames)
{
return attributeTypeNames.Any(attr => attr.Equals(BINDING_ATTR, StringComparison.InvariantCulture));
return attributeTypeNames.Any(attr => attr.Equals(BindingAttributeFullName, StringComparison.InvariantCulture));
}

public bool PreFilterType(IEnumerable<string> attributeTypeNames)
Expand All @@ -43,23 +43,27 @@ public bool PreFilterType(IEnumerable<string> attributeTypeNames)

public bool ProcessType(BindingSourceType bindingSourceType)
{
typeScopes = null;
_typeScopes = null;

if (!IsBindingType(bindingSourceType))
return false;

if (!ValidateType(bindingSourceType))
var validationResult = ValidateType(bindingSourceType);
if (!validationResult.IsValid)
{
OnValidationError(validationResult, true);
return false;
}

currentBindingSourceType = bindingSourceType;
typeScopes = GetScopes(bindingSourceType.Attributes).ToArray();
_currentBindingSourceType = bindingSourceType;
_typeScopes = GetScopes(bindingSourceType.Attributes).ToArray();
return true;
}

public void ProcessTypeDone()
{
currentBindingSourceType = null;
typeScopes = null;
_currentBindingSourceType = null;
_typeScopes = null;
}

public virtual void BuildingCompleted()
Expand Down Expand Up @@ -110,7 +114,7 @@ private bool IsStepArgumentTransformationAttribute(BindingSourceAttribute attrib

public void ProcessMethod(BindingSourceMethod bindingSourceMethod)
{
var methodScopes = typeScopes.Concat(GetScopes(bindingSourceMethod.Attributes)).ToArray();
var methodScopes = _typeScopes.Concat(GetScopes(bindingSourceMethod.Attributes)).ToArray();

ProcessStepDefinitions(bindingSourceMethod, methodScopes);
ProcessHooks(bindingSourceMethod, methodScopes);
Expand Down Expand Up @@ -174,10 +178,14 @@ private void ProcessHookAttribute(BindingSourceMethod bindingSourceMethod, Bindi
HookType hookType = GetHookType(hookAttribute);
int order = GetHookOrder(hookAttribute);

if (!ValidateHook(bindingSourceMethod, hookAttribute, hookType))
var validationResult = ValidateHook(bindingSourceMethod, hookAttribute, hookType);
if (!validationResult.IsValid)
{
OnValidationError(validationResult, true);
return;
}

var hookBinding = bindingFactory.CreateHookBinding(bindingSourceMethod.BindingMethod, hookType, scope, order);
var hookBinding = _bindingFactory.CreateHookBinding(bindingSourceMethod.BindingMethod, hookType, scope, order);

ProcessHookBinding(hookBinding);
}
Expand All @@ -192,10 +200,14 @@ private void ProcessStepArgumentTransformationAttribute(BindingSourceMethod bind
string regex = stepArgumentTransformationAttribute.TryGetAttributeValue<string>(0) ?? stepArgumentTransformationAttribute.TryGetAttributeValue<string>(nameof(StepArgumentTransformationAttribute.Regex));
string name = stepArgumentTransformationAttribute.TryGetAttributeValue<string>(nameof(StepArgumentTransformationAttribute.Name));

if (!ValidateStepArgumentTransformation(bindingSourceMethod, stepArgumentTransformationAttribute))
var validationResult = ValidateStepArgumentTransformation(bindingSourceMethod, stepArgumentTransformationAttribute);
if (!validationResult.IsValid)
{
OnValidationError(validationResult, true);
return;
}

var stepArgumentTransformationBinding = bindingFactory.CreateStepArgumentTransformation(regex, bindingSourceMethod.BindingMethod, name);
var stepArgumentTransformationBinding = _bindingFactory.CreateStepArgumentTransformation(regex, bindingSourceMethod.BindingMethod, name);

ProcessStepArgumentTransformationBinding(stepArgumentTransformationBinding);
}
Expand Down Expand Up @@ -234,61 +246,98 @@ private void ProcessStepDefinitionAttribute(BindingSourceMethod bindingSourceMet
var stepDefinitionTypes = GetStepDefinitionTypes(stepDefinitionAttribute);
string regex = stepDefinitionAttribute.TryGetAttributeValue<string>(0);

if (!ValidateStepDefinition(bindingSourceMethod, stepDefinitionAttribute))
var validationResult = ValidateStepDefinition(bindingSourceMethod, stepDefinitionAttribute);
if (!validationResult.IsValid)
{
OnValidationError(validationResult, false);
foreach (var stepDefinitionType in stepDefinitionTypes)
{
_stepDefinitionBindingBuilders.Add(new InvalidStepDefinitionBindingBuilder(
stepDefinitionType, bindingSourceMethod.BindingMethod, scope, regex, validationResult.CombinedErrorMessages));
}
return;
}

foreach (var stepDefinitionType in stepDefinitionTypes)
{
var stepDefinitionBindingBuilder = bindingFactory.CreateStepDefinitionBindingBuilder(stepDefinitionType, bindingSourceMethod.BindingMethod, scope, regex);
var stepDefinitionBindingBuilder = _bindingFactory.CreateStepDefinitionBindingBuilder(stepDefinitionType, bindingSourceMethod.BindingMethod, scope, regex);
_stepDefinitionBindingBuilders.Add(stepDefinitionBindingBuilder);
}
}

protected virtual bool OnValidationError(string messageFormat, params object[] arguments)
protected readonly struct BindingValidationResult
{
return false;
}
public static BindingValidationResult Valid = new();
public static BindingValidationResult Error(string errorMessage) => new (errorMessage);

protected virtual bool ValidateType(BindingSourceType bindingSourceType)
{
if (!bindingSourceType.IsClass && OnValidationError("Binding types must be classes: {0}", bindingSourceType))
return false;
public List<string> ErrorMessages { get; }

if (bindingSourceType.IsGenericTypeDefinition && OnValidationError("Binding types cannot be generic: {0}", bindingSourceType))
return false;
private BindingValidationResult(string errorMessage)
{
ErrorMessages = new List<string> { errorMessage };
}

return true;
private BindingValidationResult(List<string> errorMessages)
{
ErrorMessages = new List<string>(errorMessages);
}

public bool IsValid => ErrorMessages == null || ErrorMessages.Count == 0;

public string CombinedErrorMessages => string.Join(", ", ErrorMessages);

public static BindingValidationResult operator +(BindingValidationResult r1, BindingValidationResult r2)
{
if (r1.IsValid) return r2;
if (r2.IsValid) return r1;
var result = new BindingValidationResult(r1.ErrorMessages);
result.ErrorMessages.AddRange(r2.ErrorMessages);
return result;
}
}

protected virtual bool ValidateMethod(BindingSourceMethod bindingSourceMethod)
protected virtual void OnValidationError(BindingValidationResult validationResult, bool genericBindingError)
{
if (currentBindingSourceType.IsAbstract && !bindingSourceMethod.IsStatic && OnValidationError("Abstract binding types can have only static binding methods: {0}", bindingSourceMethod))
return false;

if (AsyncMethodHelper.IsAsyncVoid(bindingSourceMethod.BindingMethod) && OnValidationError("Invalid binding method '{0}': async void methods are not supported. Please use 'async Task'.", bindingSourceMethod))
return false;
//nop;
}

return true;
protected virtual BindingValidationResult ValidateType(BindingSourceType bindingSourceType)
{
var result = BindingValidationResult.Valid;
if (!bindingSourceType.IsClass)
result += BindingValidationResult.Error($"Binding types must be classes: {bindingSourceType}");
if (bindingSourceType.IsGenericTypeDefinition)
result += BindingValidationResult.Error($"Binding types cannot be generic: {bindingSourceType}");
return result;
}

protected virtual bool ValidateStepDefinition(BindingSourceMethod bindingSourceMethod, BindingSourceAttribute stepDefinitionAttribute)
protected virtual BindingValidationResult ValidateMethod(BindingSourceMethod bindingSourceMethod)
{
if (!ValidateMethod(bindingSourceMethod))
return false;
var result = BindingValidationResult.Valid;
if (_currentBindingSourceType.IsAbstract && !bindingSourceMethod.IsStatic)
result += BindingValidationResult.Error($"Abstract binding types can only have static binding methods: {bindingSourceMethod}");
if (AsyncMethodHelper.IsAsyncVoid(bindingSourceMethod.BindingMethod))
result += BindingValidationResult.Error($"Invalid binding method '{bindingSourceMethod}': async void methods are not supported. Please use 'async Task'.");

return true;
return result;
}

protected virtual bool ValidateHook(BindingSourceMethod bindingSourceMethod, BindingSourceAttribute hookAttribute, HookType hookType)
protected virtual BindingValidationResult ValidateStepDefinition(BindingSourceMethod bindingSourceMethod, BindingSourceAttribute stepDefinitionAttribute)
{
if (!ValidateMethod(bindingSourceMethod))
return false;
var result = BindingValidationResult.Valid;
result += ValidateMethod(bindingSourceMethod);
return result;
}

if (!IsScenarioSpecificHook(hookType) && !bindingSourceMethod.IsStatic &&
OnValidationError("The binding methods for before/after feature and before/after test run events must be static! {0}", bindingSourceMethod))
return false;
protected virtual BindingValidationResult ValidateHook(BindingSourceMethod bindingSourceMethod, BindingSourceAttribute hookAttribute, HookType hookType)
{
var result = BindingValidationResult.Valid;
result += ValidateMethod(bindingSourceMethod);

return true;
if (!IsScenarioSpecificHook(hookType) && !bindingSourceMethod.IsStatic)
result += BindingValidationResult.Error($"The binding methods for before/after feature and before/after test run events must be static: {bindingSourceMethod}");

return result;
}

protected bool IsScenarioSpecificHook(HookType hookType)
Expand All @@ -302,12 +351,11 @@ protected bool IsScenarioSpecificHook(HookType hookType)
hookType == HookType.AfterStep;
}

protected virtual bool ValidateStepArgumentTransformation(BindingSourceMethod bindingSourceMethod, BindingSourceAttribute stepArgumentTransformationAttribute)
protected virtual BindingValidationResult ValidateStepArgumentTransformation(BindingSourceMethod bindingSourceMethod, BindingSourceAttribute stepArgumentTransformationAttribute)
{
if (!ValidateMethod(bindingSourceMethod))
return false;

return true;
var result = BindingValidationResult.Valid;
result += ValidateMethod(bindingSourceMethod);
return result;
}

protected abstract void ProcessStepDefinitionBinding(IStepDefinitionBinding stepDefinitionBinding);
Expand Down
2 changes: 2 additions & 0 deletions TechTalk.SpecFlow/Bindings/Discovery/BindingSourceType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@ public class BindingSourceType
public bool IsNested { get; set; }

public BindingSourceAttribute[] Attributes { get; set; }

public override string ToString() => BindingType?.ToString() ?? "<null>";
}
}
Loading

0 comments on commit 99ce6a5

Please sign in to comment.