Specflow: Bug: Value retriever based on column names not retrieving value

Created on 13 Nov 2020 · 6Comments · Source: SpecFlowOSS/SpecFlow

SpecFlow Version:

[X] 3.5
[ ] 3.4
[ ] 3.3
[ ] 3.1
[ ] 3.0
[ ] 2.4
[ ] 2.3
[ ] 2.2
[ ] 2.1
[ ] 2.0
[ ] 1.9

Used Test Runner

[ ] SpecFlow+Runner
[X] MSTest
[ ] NUnit
[X] Xunit

Version number:

Xunit 2.4.1
MSTest 2.1.2

Project Format of the SpecFlow project

[ ] Classic project format using packages.config
[ ] Classic project format using <PackageReference> tags
[X] Sdk-style project format

.feature.cs files are generated using

[ ] SpecFlow.Tools.MsBuild.Generation NuGet package
[ ] SpecFlowSingleFileGenerator custom tool

Does not apply, see commit.

Visual Studio Version

[X] VS 2019
[ ] VS 2017
[ ] VS 2015

Visual Studio 2019 - 16.8.0

'Enable SpecFlowSingleFileGenerator Custom Tool' option in Visual Studio extension settings

[ ] Enabled
[ ] Disabled

Does not apply.

Are the latest Visual Studio updates installed?

[X] Yes
[ ] No, I use Visual Studio version <Major>.<Minor>.<Patch>

.NET Framework:

[X] >= .NET 4.5
[ ] before .NET 4.5
[ ] .NET Core 2.0
[ ] .NET Core 2.1
[ ] .NET Core 2.2
[ ] .NET Core 3.0
[ ] .NET Core 3.1
[X] .NET 5.0

Test Execution Method:

[X] Visual Studio Test Explorer
[ ] TFS/VSTS/Azure DevOps – Task – PLEASE SPECIFY THE NAME OF THE TASK
[X] Command line – dotnet test

<SpecFlow> Section in app.config or content of specflow.json

Does not apply, see commit.

Issue Description

When using a custom IValueRetriever to map a specific column names to a specific property, Retrieve is not being called while CanRetrieve returned true. This was unexpected to me.

Diving deeper into the Specflow sources to find test cases I noticed that there were testcases, but non of them were covering this scenario. I added a test case in this commit:

Is this a feature that should be working in the way I suspect, or is this indeed a bug?

If this is considered a bug: the issue is caused in file TEHelper.cs by GetMembersThatNeedToBeSet, which is checking the CanResolve but also requiring the column name to match exactly or on an alias. Which won't be the case in this scenario, resulting in Resolve not being called.

Potential workaround: use an TableAliasesAttribute, if possible (eg. not instantiating third party object.)

Steps to Reproduce

Create an IValueRetriever implementation.
Use specific column names to map them to a complex type. Check for colum and type match in CanRetrieve.
The implementation of Retrieve is irrelevant, as it is not being called.
Create a feature file (or unittest) to reproduce the issue, where the colum name is used. See repro project for example.

Example:
```C#
public class ParentItem
{
public string Name { get; set; }

public ChildItem DutchItem { get; set; }

public ChildItem EnglishItem { get; set; }

}

public class ChildItem
{
public string ItemName { get; set; }

public string Language { get; set; }

}

///

/// Retrieves a ChildItem-objects for column's "Dutch name" and "English name" and maps them
/// to the corresponding "DutchItem" and "EnglishItem" properties of a ParentItem-object.
///

public class ChildItemValueRetriever : IValueRetriever
{
private static readonly IEnumerable TypesForWhichIRetrieveValues = new Type[] { typeof(ChildItem) };

private static readonly IDictionary<string, string> ColumnNamesForWhichIRetrieveValues =
    new Dictionary<string, string>
    {
        { "Dutch name", "nl-NL" },
        { "English name", "en-US" }
    };

public bool CanRetrieve(KeyValuePair<string, string> keyValuePair, Type targetType, Type type)
{
    return TypesForWhichIRetrieveValues.Contains(type) 
        && ColumnNamesForWhichIRetrieveValues.ContainsKey(keyValuePair.Key);
}

public object Retrieve(KeyValuePair<string, string> keyValuePair, Type targetType, Type propertyType)
{
    return Parse(keyValuePair.Key, keyValuePair.Value);
}

public static ChildItem Parse(string columnName, string columnValue)
{
    return new ChildItem
    {
        ItemName = columnValue,
        Language = ColumnNamesForWhichIRetrieveValues[columnName]
    };
}

}
```

Repro Project

See the following commit on my SpecFlow fork. It is based on the example above.
https://github.com/timvandenhof/SpecFlow/commit/dad342db4a67138714c8b8f598a2e8f338d4b6c6

Bug Assist Runtime easy medium OSS Iteration Candidate

Source

timvandenhof

All 6 comments

@timvandenhof Could you send this failing tests as a PR? That would be lesser work for us to start debugging. Thanks!

SabotageAndi on 13 Nov 2020

@SabotageAndi Sure, I just opend draft PR #2207 for this issue.
PR contains test scenario for this issue, not a fix yet.
Also updated existing testscenario's to not use the deprecated Register methods.

timvandenhof on 13 Nov 2020

👍1

Thanks! Having the test should it make it easier to debug to see what is happening

SabotageAndi on 13 Nov 2020

👍1

I'm suspecting this part in TEHelper.cs, method GetMembersThatNeedToBeSet:

C# var properties = (from property in type.GetProperties() from row in table.Rows where TheseTypesMatch(type, property.PropertyType, row) && (IsMemberMatchingToColumnName(property, row.Id()) || IsMatchingAlias(property, row.Id())) select new MemberHandler { Type = type, Row = row, MemberName = property.Name, PropertyType = property.PropertyType, Setter = (i, v) => property.SetValue(i, v, null) }).ToList();

TheseTypesMatch will return true because there is a IValueRetriever available, however they are not IsMemberMatchingToColumnName and not IsMatchingAlias. Removing those checks will break other existing tests.

timvandenhof on 16 Nov 2020

This is not a bug. Using TableAliasesattribute is the solution, not the workaround.
In your case, please use "Dutch name" and "English name" aliases.

    public class ParentItem
    {
        public string Name { get; set; }

        [TableAliases("Dutch name")]
        public ChildItem DutchItem { get; set; }

        [TableAliases("English name")]
        public ChildItem EnglishItem { get; set; }
    }